From a75641f4b263dcbed1bf0271ca3a8f67d4898574 Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:23:30 -0700 Subject: [PATCH 01/11] Add inline image format helpers --- .../apps/files/inline_image.py | 163 ++++++++++++++++++ packages/apps/tests/test_inline_image.py | 129 ++++++++++++++ 2 files changed, 292 insertions(+) create mode 100644 packages/apps/src/microsoft_teams/apps/files/inline_image.py create mode 100644 packages/apps/tests/test_inline_image.py diff --git a/packages/apps/src/microsoft_teams/apps/files/inline_image.py b/packages/apps/src/microsoft_teams/apps/files/inline_image.py new file mode 100644 index 00000000..280fb787 --- /dev/null +++ b/packages/apps/src/microsoft_teams/apps/files/inline_image.py @@ -0,0 +1,163 @@ +""" +Copyright (c) Microsoft Corporation. All rights reserved. +Licensed under the MIT License. +""" + +import re +from dataclasses import dataclass +from typing import Literal, Optional, cast +from urllib.parse import urlsplit + +InlineImageFormat = Literal["png", "jpeg", "gif", "webp"] +""" +The image formats the SDK gives an extension and a specific MIME type. + +PNG, JPEG, and WebP are named and typed with confidence. +GIF is not: Teams saves a pasted GIF as a static PNG, so it is listed as `.gif` and `image/gif` but downloads as a PNG. +Other formats, such as BMP and AVIF, are not given an extension or a specific MIME type. +""" + +_INLINE_IMAGE_FORMATS: dict[str, tuple[str, str]] = { + "png": ("png", "image/png"), + "jpeg": ("jpg", "image/jpeg"), + "gif": ("gif", "image/gif"), + "webp": ("webp", "image/webp"), +} + +_OCTET_STREAM = "application/octet-stream" + +_INLINE_IMAGE_PATH = re.compile(r"/v3/attachments/([A-Za-z0-9_.-]+)/views/original$", re.IGNORECASE) +""" +Matches the path of an inline image hosted by the Bot API, capturing its attachment id. + +Pattern: the URL's path ends in `/v3/attachments//views/original`, so the same text in a query string or +fragment does not match. +The id becomes part of the file name, so it is limited to letters, digits, `_`, `.`, and `-`, which keeps path +separators out of it. +""" + +_IMG_TAG = re.compile(r"]*>", re.IGNORECASE | re.ASCII) + +_ATTRIBUTE = re.compile(r"(? Optional[str]: + """ + The attachment id of an inline image, taken from its `content_url`, or `None` when the URL is not an inline + image location. + """ + try: + path = urlsplit(content_url).path + except ValueError: + return None + + match = _INLINE_IMAGE_PATH.search(path) + return match.group(1) if match else None + + +def parse_inline_image_hints(html: str) -> dict[str, str]: + """ + Map each inline image id found in a message's HTML to its format hint. + + The hint is the `itemscope` attribute of an `` whose `itemtype` marks it as an inline image, keyed by its + `itemid`. + """ + hints: dict[str, str] = {} + + # Each start tag + for tag in _IMG_TAG.findall(html): + attributes: dict[str, str] = {} + + # Each name="value", capturing the name and whichever quoted value matched. + # A name starts only where the previous character is not part of a name, so a long run of name characters is + # scanned once rather than once per position. + for match in _ATTRIBUTE.finditer(tag): + double_quoted, single_quoted = match.group(2), match.group(3) + attributes[match.group(1).lower()] = double_quoted if double_quoted is not None else single_quoted + + id = attributes.get("itemid") + scope = attributes.get("itemscope") + + if attributes.get("itemtype") == AMS_IMAGE_ITEMTYPE and id and scope: + hints[id] = scope + + return hints + + +def format_from_hint(hint: Optional[str]) -> Optional[InlineImageFormat]: + """The format named by an `itemscope` hint, or `None` when it is not one of `InlineImageFormat`.""" + normalized = hint.strip().lower() if hint else None + + if normalized == "jpg": + return "jpeg" + + if normalized in _INLINE_IMAGE_FORMATS: + return cast(InlineImageFormat, normalized) + + return None + + +def format_from_content_type(content_type: Optional[str]) -> Optional[InlineImageFormat]: + """The format named by an `image/*` content type, or `None` when it is not one of `InlineImageFormat`.""" + # "image/" then the subtype (letters, digits, ".", "+" or "-"), capturing the subtype. + match = _IMAGE_CONTENT_TYPE.match(content_type.strip() if content_type else "") + return format_from_hint(match.group(1) if match else None) + + +def sniff_image_format(data: bytes) -> Optional[InlineImageFormat]: + """ + The format the bytes actually are, read from their leading magic bytes, or `None` when they are none of + `InlineImageFormat`. + + The platform can return bytes in a different format from the hint (a GIF downloads as a static PNG), so the bytes + are the stronger signal. + """ + + def starts_with(signature: bytes, offset: int = 0) -> bool: + return len(data) >= offset + len(signature) and data[offset : offset + len(signature)] == signature + + if starts_with(b"\x89PNG\r\n\x1a\n"): + return "png" + + if starts_with(b"\xff\xd8\xff"): + return "jpeg" + + if starts_with(b"GIF87a") or starts_with(b"GIF89a"): + return "gif" + + # "RIFF" at byte 0 and "WEBP" at byte 8; bytes 4-7 are the file size. + if starts_with(b"RIFF") and starts_with(b"WEBP", 8): + return "webp" + + return None + + +def inline_image_naming(base_name: str, format: Optional[InlineImageFormat] = None) -> InlineImageNaming: + """ + The filename, extension, and MIME type for an inline image of the given format. + + An unknown format gets no extension and `application/octet-stream` rather than a guess. + """ + if not format: + return InlineImageNaming(name=base_name, extension=None, content_type=_OCTET_STREAM) + + extension, content_type = _INLINE_IMAGE_FORMATS[format] + return InlineImageNaming(name=f"{base_name}.{extension}", extension=extension, content_type=content_type) diff --git a/packages/apps/tests/test_inline_image.py b/packages/apps/tests/test_inline_image.py new file mode 100644 index 00000000..ccdb394d --- /dev/null +++ b/packages/apps/tests/test_inline_image.py @@ -0,0 +1,129 @@ +""" +Copyright (c) Microsoft Corporation. All rights reserved. +Licensed under the MIT License. +""" + +# pyright: basic + +import time + +from microsoft_teams.apps.files.inline_image import ( + AMS_IMAGE_ITEMTYPE, + format_from_hint, + inline_image_id, + parse_inline_image_hints, + sniff_image_format, +) + + +def test_sniff_recognizes_gif87a_as_well_as_gif89a() -> None: + assert sniff_image_format(b"GIF87a") == "gif" + + +def test_sniff_does_not_treat_a_riff_container_other_than_webp_as_webp() -> None: + wav = b"RIFF\x00\x00\x00\x00WAVE" + + assert sniff_image_format(wav) is None + + +def test_sniff_does_not_match_a_signature_longer_than_the_bytes() -> None: + assert sniff_image_format(b"\x89PN") is None + assert sniff_image_format(b"RIFF") is None + assert sniff_image_format(b"") is None + + +def test_format_from_hint_accepts_jpg_and_any_case_and_rejects_an_unknown_name() -> None: + assert format_from_hint("JPG") == "jpeg" + assert format_from_hint(" Png ") == "png" + assert format_from_hint("bmp") is None + assert format_from_hint(None) is None + + +def test_inline_image_id_reads_the_id_only_from_an_inline_image_location() -> None: + assert ( + inline_image_id("https://smba.trafficmanager.net/amer/v3/attachments/0-wus-d1-abc/views/original") + == "0-wus-d1-abc" + ) + assert inline_image_id("https://smba.trafficmanager.net/amer/v3/attachments/0-wus-d1-abc/views/thumbnail") is None + + +def test_inline_image_id_rejects_an_id_that_could_carry_a_path_separator_into_the_file_name() -> None: + assert inline_image_id("https://smba.trafficmanager.net/amer/v3/attachments/a\\..\\..\\evil/views/original") is None + assert inline_image_id("https://smba.trafficmanager.net/amer/v3/attachments/a%5C..%5Cevil/views/original") is None + + +def test_inline_image_id_reads_the_id_only_from_the_path_not_from_a_query_string_or_fragment() -> None: + assert ( + inline_image_id( + "https://smba.trafficmanager.net/amer/v3/conversations/x/members?x=/v3/attachments/a/views/original" + ) + is None + ) + assert ( + inline_image_id( + "https://smba.trafficmanager.net/amer/v3/conversations/x/members#/v3/attachments/a/views/original" + ) + is None + ) + assert inline_image_id("https://smba.trafficmanager.net/amer/v3/attachments/a/views/original?x=1") == "a" + + +def test_hints_read_double_quoted_single_quoted_and_mixed_attribute_values() -> None: + assert parse_inline_image_hints(f'') == {"a": "png"} + assert parse_inline_image_hints(f"") == {"a": "png"} + assert parse_inline_image_hints(f'') == { + "a": "png" + } + + +def test_hints_do_not_read_an_attribute_from_inside_the_value_of_another_attribute() -> None: + assert parse_inline_image_hints( + f'a itemscope=\'gif\'' + ) == {"a": "png"} + assert parse_inline_image_hints( + f'\'a' + ) == {"a": "png"} + + +def test_hints_read_a_tag_holding_a_long_run_of_name_characters_in_linear_time() -> None: + started = time.monotonic() + + html = f'' + assert parse_inline_image_hints(html) == {"a": "png"} + assert time.monotonic() - started < 0.25 + + +def test_hints_match_tag_and_attribute_names_in_any_case() -> None: + assert parse_inline_image_hints(f'') == {"a": "png"} + + +def test_hints_map_each_inline_image_in_the_message_to_its_own_hint() -> None: + html = ( + f'

' + f'

' + ) + + assert parse_inline_image_hints(html) == {"a": "png", "b": "jpeg"} + + +def test_hints_ignore_an_image_that_is_not_marked_as_an_inline_image() -> None: + assert parse_inline_image_hints('') == {} + assert parse_inline_image_hints('') == {} + + +def test_hints_read_nothing_from_an_unquoted_or_bare_attribute() -> None: + assert parse_inline_image_hints(f'') == {} + assert parse_inline_image_hints(f'') == {} + + +def test_hints_do_not_decode_character_references() -> None: + encoded = parse_inline_image_hints(f'') + + assert encoded == {"a": "png"} + assert format_from_hint(encoded["a"]) is None + assert ( + parse_inline_image_hints( + '' + ) + == {} + ) From 05a38fa5d015f06bfd150caaf6341f7b23738627 Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:27:05 -0700 Subject: [PATCH 02/11] Add inline image download route and errors --- .../microsoft_teams/apps/files/__init__.py | 7 +- .../microsoft_teams/apps/files/download.py | 223 +++++++++- .../apps/files/downloaded_file.py | 18 +- .../src/microsoft_teams/apps/files/errors.py | 80 +++- .../apps/files/incoming_file.py | 79 +++- .../src/microsoft_teams/apps/files/types.py | 11 + .../apps/tests/test_files_download_inline.py | 396 ++++++++++++++++++ 7 files changed, 789 insertions(+), 25 deletions(-) create mode 100644 packages/apps/tests/test_files_download_inline.py diff --git a/packages/apps/src/microsoft_teams/apps/files/__init__.py b/packages/apps/src/microsoft_teams/apps/files/__init__.py index 34c30dda..30e64c57 100644 --- a/packages/apps/src/microsoft_teams/apps/files/__init__.py +++ b/packages/apps/src/microsoft_teams/apps/files/__init__.py @@ -11,19 +11,24 @@ FileError, FileScopeNotSupportedError, FileUrlExpiredError, + InlineImageAccessError, + InlineImageTransformationError, ) from .files_accessor import FilesAccessor from .incoming_file import IncomingFile -from .types import FileSource +from .types import AttachmentStyle, FileSource __all__ = [ "FileSource", + "AttachmentStyle", "FileActor", "FileError", "FileAccessError", "FileCredentialError", "FileScopeNotSupportedError", "FileUrlExpiredError", + "InlineImageAccessError", + "InlineImageTransformationError", "DownloadedFile", "IncomingFile", "FilesAccessor", diff --git a/packages/apps/src/microsoft_teams/apps/files/download.py b/packages/apps/src/microsoft_teams/apps/files/download.py index d7a210bf..69772a18 100644 --- a/packages/apps/src/microsoft_teams/apps/files/download.py +++ b/packages/apps/src/microsoft_teams/apps/files/download.py @@ -11,12 +11,23 @@ from contextlib import asynccontextmanager from dataclasses import dataclass from typing import Any, Optional, cast +from urllib.parse import urlsplit import httpx from microsoft_teams.api import ConversationType - -from .errors import FileAccessError, FileActor, FileCredentialError, FileScopeNotSupportedError, FileUrlExpiredError +from microsoft_teams.common.http.client_token import Token, resolve_token + +from .errors import ( + FileAccessError, + FileActor, + FileCredentialError, + FileScopeNotSupportedError, + FileUrlExpiredError, + InlineImageAccessError, + InlineImageTransformationError, +) from .graph_share import build_drive_item_content_url +from .types import AttachmentStyle logger = logging.getLogger(__name__) @@ -43,6 +54,13 @@ class FileFetchTarget: content_type: Optional[str] = None """MIME type reported by the incoming file, used as a fallback when the response omits one.""" + attachment_style: Optional[AttachmentStyle] = None + """ + How the file was attached; see `AttachmentStyle`. + An `inlineImage` is read from `content_url` with the Bot API credential, in any scope. + Defaults to `file`. + """ + @dataclass class GraphCredential: @@ -66,6 +84,88 @@ class GraphCredential: """Graph host root, e.g. `https://graph.microsoft.com`. The API version is appended at the point of use.""" +@dataclass +class BotApiCredential: + """ + The Bot API token for reading inline images, and the identity it belongs to. + + It is the turn's API client token, so an agentic identity reads as itself. + Resolved at fetch time, so a turn that never downloads an inline image never acquires one. + """ + + actor: FileActor + """Which identity this credential belongs to.""" + + token: Token + """Resolves the bearer token, or `None` when no credential is available.""" + + service_url: Optional[str] = None + """ + The activity's `service_url`, the base the turn's replies are sent to. + + Carried with the token so the token is never sent without its allowed destination. + An inline image under a different URL is refused before the token is resolved. + When absent, every inline image is refused. + """ + + +_SINGLE_DOT_SEGMENTS = frozenset({".", "%2e"}) +_DOUBLE_DOT_SEGMENTS = frozenset({"..", ".%2e", "%2e.", "%2e%2e"}) + + +def _resolve_dot_segments(path: str) -> str: + """ + Resolve `.` and `..` segments in a URL path. + + `urlsplit` keeps them verbatim, so a path such as `/amer/../evil` would otherwise compare as being under `/amer`. + + A percent-encoded dot counts as a dot, following the URL standard, so `%2e%2e` climbs out just as `..` does. + `%2f` is left alone, so a segment such as `..%2f..%2f` stays literal. + """ + resolved: list[str] = [] + + for segment in path.lstrip("/").split("/"): + folded = segment.lower() + if folded in _SINGLE_DOT_SEGMENTS: + continue + if folded in _DOUBLE_DOT_SEGMENTS: + if resolved: + resolved.pop() + continue + resolved.append(segment) + + return "/" + "/".join(resolved) + + +def is_under_service_url(url: str, service_url: Optional[str]) -> bool: + """ + Whether `url` is under `service_url`: both https, the same origin, and a path starting with the service URL's + path. + + Both are parsed first, and their dot segments are resolved, so a path cannot climb out of the service URL's path. + """ + if not service_url: + return False + + try: + target = urlsplit(url) + base = urlsplit(service_url) + origins_match = (target.scheme.lower(), target.hostname, target.port) == ( + base.scheme.lower(), + base.hostname, + base.port, + ) + except ValueError: + return False + + if base.scheme.lower() != "https" or not origins_match: + return False + + # Normalizes to exactly one trailing `/` so a path such as `/amer` cannot match `/americas`. + base_path = _resolve_dot_segments(base.path).rstrip("/") + "/" + return _resolve_dot_segments(target.path).lower().startswith(base_path.lower()) + + @dataclass class OpenedFileStream: """A freshly opened, single-consumption byte stream plus the metadata resolved while opening it.""" @@ -102,6 +202,7 @@ async def open_file_stream( prior_fetch_succeeded: bool = False, client: Optional[httpx.AsyncClient] = None, credential: Optional[GraphCredential] = None, + bot_api_credential: Optional[BotApiCredential] = None, ) -> AsyncGenerator[OpenedFileStream]: """ Open a byte stream for an inbound file, keyed on its conversation scope so every scope's receive path extends this @@ -111,6 +212,12 @@ async def open_file_stream( agentic user, each carrying just a `content_url`, so they open only through Graph, as `is_graph_route_open` decides. Everything else raises `FileScopeNotSupportedError`. """ + # Inline images live on the Bot API, so neither file route nor its scope restriction applies. + if target.attachment_style == "inlineImage": + async with _open_inline_image_stream(target, client=client, credential=bot_api_credential) as opened: + yield opened + return + if target.scope == "personal": async with _open_personal_file_stream( target, prior_fetch_succeeded=prior_fetch_succeeded, client=client, credential=credential @@ -295,14 +402,88 @@ async def _open_graph_file_stream( await http.aclose() +@asynccontextmanager +async def _open_inline_image_stream( + target: FileFetchTarget, + *, + client: Optional[httpx.AsyncClient], + credential: Optional[BotApiCredential], +) -> AsyncGenerator[OpenedFileStream]: + """ + Fetch an inline image's bytes from the Bot API with the turn's Bot API token. + + The token is sent only to a URL under `service_url`. + """ + url = target.content_url + + if not url: + raise RuntimeError("cannot download inline image: no content URL is available") + + if not url.lower().startswith("https://"): + raise RuntimeError("cannot download inline image: content URL must use https") + + if not is_under_service_url(url, credential.service_url if credential else None): + raise RuntimeError("cannot download inline image: content URL is not under the activity's service URL") + + actor = credential.actor if credential else None + token = (await resolve_token(credential.token) if credential else None) or "" + + # `resolve_token` passes a blank value through, and a blank bearer token is no token at all. + if not token.strip(): + raise RuntimeError("cannot download inline image: no Bot API token is available") + + logger.debug(f"reading inline image bytes from the Bot API as '{actor}'") + + owns_client = client is None + http = client or httpx.AsyncClient() + + try: + request = http.build_request("GET", url, headers={"Authorization": f"Bearer {token}"}) + response = await _send_following_https_redirects(http, request) + + try: + if response.status_code == 415: + body = await _read_error_body(response) + raise InlineImageTransformationError( + response.status_code, + _extract_service_error_code(body), + _extract_service_error(body), + actor, + ) + + if response.status_code in (401, 403): + raise InlineImageAccessError(response.status_code, actor, await _read_service_error(response)) + + if not response.is_success: + details = await _read_service_error(response) + message = ( + f"failed to download inline image as '{actor or 'app'}': " + f"{response.status_code} {response.reason_phrase}" + ).strip() + raise RuntimeError(f"{message} ({details})" if details else message) + + content_type = response.headers.get("content-type") or target.content_type or "application/octet-stream" + yield OpenedFileStream(chunks=response.aiter_bytes(), source_url=url, content_type=content_type) + finally: + await response.aclose() + finally: + if owns_client: + await http.aclose() + + async def _read_service_error(response: httpx.Response) -> Optional[str]: + """Pull the human-readable part out of a service error body, falling back to the raw text.""" + return _extract_service_error(await _read_error_body(response)) + + +async def _read_error_body(response: httpx.Response) -> Optional[str]: """ - Pull the human-readable part out of a Graph error envelope, falling back to the raw text. + Read an error response's text, once. - Graph replies `{"error": {"code", "message"}}`, but a 401 can also come from the edge as HTML, so this must not - assume JSON. Reads at most `_ERROR_BODY_LIMIT` bytes rather than buffering the whole body: this runs on a stream - the SDK does not size, and the text only ever lands in an exception message. A body large enough to be cut off - will not parse as JSON, which is fine, because the raw fallback is what a body that shape deserves anyway. + Reads at most `_ERROR_BODY_LIMIT` bytes rather than buffering the whole body: this runs on a stream the SDK does + not size, and the text only ever lands in an exception message. A body large enough to be cut off will not parse + as JSON, which is fine, because the raw fallback is what a body that shape deserves anyway. A stream can be + consumed only once, so a caller that wants both the message and the code parses this one string twice. """ try: collected = bytearray() @@ -312,10 +493,21 @@ async def _read_service_error(response: httpx.Response) -> Optional[str]: collected.extend(chunk[: _ERROR_BODY_LIMIT - len(collected)]) if len(collected) >= _ERROR_BODY_LIMIT: break - raw = bytes(collected).decode("utf-8", errors="replace") + return bytes(collected).decode("utf-8", errors="replace") except Exception: # noqa: BLE001 - diagnostics must never mask the error being raised return None + +def _extract_service_error(raw: Optional[str]) -> Optional[str]: + """ + The human-readable part of a Graph error envelope, falling back to the raw text. + + Graph replies `{"error": {"code", "message"}}`, but a 401 can also come from the edge as HTML, so this must not + assume JSON. + """ + if not raw: + return None + try: envelope: Any = json.loads(raw) error = cast(dict[str, Any], envelope).get("error") if isinstance(envelope, dict) else None @@ -330,6 +522,21 @@ async def _read_service_error(response: httpx.Response) -> Optional[str]: return _truncate(raw) +def _extract_service_error_code(raw: Optional[str]) -> Optional[str]: + """The `error.code` from a JSON error envelope, or `None` when the body is not one.""" + if not raw: + return None + + try: + envelope: Any = json.loads(raw) + except ValueError: + return None + + error = cast(dict[str, Any], envelope).get("error") if isinstance(envelope, dict) else None + code = cast(dict[str, Any], error).get("code") if isinstance(error, dict) else None + return code.strip() if isinstance(code, str) and code.strip() else None + + def _truncate(text: str) -> Optional[str]: collapsed = " ".join(text.split()) if not collapsed: diff --git a/packages/apps/src/microsoft_teams/apps/files/downloaded_file.py b/packages/apps/src/microsoft_teams/apps/files/downloaded_file.py index df19c160..fa74a98b 100644 --- a/packages/apps/src/microsoft_teams/apps/files/downloaded_file.py +++ b/packages/apps/src/microsoft_teams/apps/files/downloaded_file.py @@ -23,12 +23,22 @@ class DownloadedFile: """The file bytes, buffered from `stream()` read to completion.""" content_type: str - """MIME type resolved from the download response header, or the incoming file's metadata type if the - response omits one. Falls back to `application/octet-stream` when neither provides a type, so this is - never empty.""" + """ + MIME type resolved from the download response header, or the incoming file's metadata type if the response omits + one. + Falls back to `application/octet-stream` when neither provides a type, so this is never empty. + For an inline image, the response header is ignored and the type is detected from the bytes. + If the bytes are not PNG, JPEG, GIF, or WebP, it falls back to `IncomingFile.content_type`. + """ filename: str - """Resolved filename.""" + """ + Resolved filename. + For an inline image, the extension follows `content_type`, meaning the filename can differ from + `IncomingFile.name`. + For an uploaded file, the name comes from the sender and can contain path separators, so reduce it to its last + segment (for example with `ntpath.basename(name)`) before using it in a local path. + """ source_url: str """The URL the bytes were actually fetched from.""" diff --git a/packages/apps/src/microsoft_teams/apps/files/errors.py b/packages/apps/src/microsoft_teams/apps/files/errors.py index a15b2ef6..4c95a005 100644 --- a/packages/apps/src/microsoft_teams/apps/files/errors.py +++ b/packages/apps/src/microsoft_teams/apps/files/errors.py @@ -18,8 +18,8 @@ class FileError(Exception): """ - Base class for the diagnosable failures on the inbound-file path: an expired URL, an unsupported scope, and a - refused Graph read. + Base class for the diagnosable failures on the inbound-file path: an expired URL, an unsupported scope, a + missing Graph credential, a refused Graph read, and an inline image that was refused or could not be converted. Lets a caller catch those with one `except` clause, so a new one can be added without callers changing. A transport or service failure the SDK cannot attribute, such as a Graph 5xx, is not one of these and surfaces as @@ -153,6 +153,82 @@ def __init__( self.details = details +class InlineImageTransformationError(FileError): + """ + Raised when the platform could not convert an inline image for download, answering `415`. + + The same image sent as a file attachment downloads normally. + """ + + status: int + """The HTTP status, `415`.""" + + code: Optional[str] + """The service's error code, such as `TransformationFailed`, when the response carried one.""" + + details: Optional[str] + """The service's message, verbatim and truncated, when present.""" + + actor: Optional[FileActor] + """The identity the fetch was attempted as.""" + + def __init__( + self, + status: int, + code: Optional[str] = None, + details: Optional[str] = None, + actor: Optional[FileActor] = None, + ) -> None: + reason = f"status {status}, code {code}" if code else f"status {status}" + message = ( + f"cannot download inline image: the platform could not convert it for download ({reason}). Some image " + "formats, such as BMP and AVIF, cannot be converted by the platform, so Teams shows a placeholder, and " + "they cannot be downloaded. Ask the user to send the image as a file attachment instead" + ) + if details: + message = f"{message} (service said: {details})" + super().__init__(message) + self.status = status + self.code = code + self.details = details + self.actor = actor + + +class InlineImageAccessError(FileError): + """ + Raised when the Bot API refused the token presented for an inline image, answering `401` or `403`. + + Inline images are read with the same Bot API token the app uses to reply in that conversation (an agentic user + token for agentic identities), not Graph, so this is distinct from `FileAccessError`. + """ + + status: int + """`401` means the token itself was rejected; `403` means the identity is not allowed to read the image.""" + + actor: Optional[FileActor] + """The identity the fetch was attempted as.""" + + details: Optional[str] + """The service's message, verbatim and truncated, when present.""" + + def __init__( + self, + status: int, + actor: Optional[FileActor] = None, + details: Optional[str] = None, + ) -> None: + as_who = _describe_actor(actor) if actor else "the identity used" + message = ( + f"cannot download inline image: the Bot API token presented for {as_who} was rejected with status {status}" + ) + if details: + message = f"{message} (service said: {details})" + super().__init__(message) + self.status = status + self.actor = actor + self.details = details + + def _describe_actor(actor: FileActor) -> str: """ Name the identity in prose. Exhaustive on purpose: a new `FileActor` must fail type checking here rather than diff --git a/packages/apps/src/microsoft_teams/apps/files/incoming_file.py b/packages/apps/src/microsoft_teams/apps/files/incoming_file.py index bbce29da..39bfc2b1 100644 --- a/packages/apps/src/microsoft_teams/apps/files/incoming_file.py +++ b/packages/apps/src/microsoft_teams/apps/files/incoming_file.py @@ -11,9 +11,17 @@ import httpx from microsoft_teams.api import ConversationType -from .download import FileFetchTarget, GraphCredential, OpenedFileStream, collect_stream, open_file_stream +from .download import ( + BotApiCredential, + FileFetchTarget, + GraphCredential, + OpenedFileStream, + collect_stream, + open_file_stream, +) from .downloaded_file import DownloadedFile -from .types import FileSource +from .inline_image import inline_image_naming, sniff_image_format +from .types import AttachmentStyle, FileSource class IncomingFile: @@ -32,23 +40,36 @@ class IncomingFile: correlation, dedup and logging, but not for retrieval: the Graph fetch resolves bytes from `content_url` through `/shares`, and this value arrives as a GUID, which is a SharePoint `listItemUniqueId` shape rather than a Graph `driveItem.id`. Present only when the wire provided it. + Always `None` for inline images, which are not stored in OneDrive or SharePoint. """ name: str - """Display name including extension when known.""" + """ + Display name including extension when known. + An inline image has no name, so it is named `image-`. + An extension is added only for PNG, JPEG, GIF, or WebP. + Teams saves a pasted GIF as a static PNG, so the SDK cannot name or type it with confidence: it is listed as + `.gif` and `image/gif` but downloads as a PNG. + """ content_type: Optional[str] """ - The file's MIME type when the source provides one. Always `None` for `botActivity` files: a `file.download.info` - attachment carries no MIME type, only the `file_type` extension surfaced as `extension`. Populated for sources - that do carry one, such as a `graph` drive item. To learn the type of the bytes you actually received, read - `DownloadedFile.content_type`, which is resolved from the download response. + The file's MIME type when the source provides one. Always `None` for `botActivity` file attachments: a + `file.download.info` attachment carries no MIME type, only the `file_type` extension surfaced as `extension`. + Populated for sources that do carry one, such as a `graph` drive item. To learn the type of the bytes you actually + received, read `DownloadedFile.content_type`, which is resolved from the download response. + + For an inline image, it is the type named by the format hint when that is PNG, JPEG, GIF, or WebP, otherwise + `application/octet-stream`. + Teams saves a pasted GIF as a static PNG, so the SDK cannot name or type it with confidence: it is listed as + `.gif` and `image/gif` but downloads as a PNG. """ extension: Optional[str] """ File extension without the dot (e.g. `pdf`), taken from the platform-supplied `file_type`. Absent when the wire omits it. + For an inline image, it is taken from `name` and is absent when the name has none. """ scope: ConversationType @@ -57,16 +78,27 @@ class IncomingFile: source: FileSource """Where the SDK found the file. Only `botActivity` is produced today.""" + attachment_style: AttachmentStyle + """ + How the file was attached: sent as a file, or pasted inline as an image. + See `AttachmentStyle`. + """ + content_url: Optional[str] """ Browsable URL to the file in OneDrive/SharePoint, as sent on the attachment's `content_url`. Not fetchable for bytes despite the name, but it is the locator a Graph `/shares` resolution keys off; bytes come from `download()` or `stream()`. + For an inline image, it is the image's Bot API location instead. """ raw: Any - """The raw underlying attachment/graph object for escape-hatch access.""" + """ + The raw underlying attachment/graph object for escape-hatch access. + For an inline image, only the image attachment is included; the HTML attachment that supplies its format hint + stays in `ctx.activity.attachments`. + """ def __init__( self, @@ -77,11 +109,14 @@ def __init__( unique_id: Optional[str] = None, content_type: Optional[str] = None, extension: Optional[str] = None, + attachment_style: Optional[AttachmentStyle] = None, content_url: Optional[str] = None, raw: Any = None, + inline_image_base_name: Optional[str] = None, download_url: Optional[str] = None, client: Optional[httpx.AsyncClient] = None, credential: Optional[GraphCredential] = None, + bot_api_credential: Optional[BotApiCredential] = None, ) -> None: self.name = name self.scope = scope @@ -89,11 +124,14 @@ def __init__( self.unique_id = unique_id self.content_type = content_type self.extension = extension + self.attachment_style = attachment_style or "file" self.content_url = content_url self.raw = raw + self._inline_image_base_name = inline_image_base_name self._download_url = download_url self._client = client self._credential = credential + self._bot_api_credential = bot_api_credential self._prior_fetch_succeeded = False async def stream(self) -> AsyncIterator[bytes]: @@ -114,10 +152,11 @@ async def download(self) -> DownloadedFile: """ async with self._open() as opened: data = await collect_stream(opened.chunks) + filename, content_type = self._resolve_downloaded_type(data, opened.content_type) return DownloadedFile( bytes=data, - content_type=opened.content_type, - filename=self.name, + content_type=content_type, + filename=filename, source_url=opened.source_url, ) @@ -143,6 +182,24 @@ async def save_as(self, path: str) -> None: finally: await asyncio.to_thread(file.close) + def _resolve_downloaded_type(self, data: bytes, response_content_type: str) -> tuple[str, str]: + """ + The filename and MIME type for downloaded bytes. + + For an inline image, recognized bytes decide, then the hint-derived name and type, because the platform can + return a different format from the hint (a GIF downloads as a static PNG). + """ + if self.attachment_style != "inlineImage": + return self.name, response_content_type + + sniffed = sniff_image_format(data) + + if sniffed and self._inline_image_base_name: + naming = inline_image_naming(self._inline_image_base_name, sniffed) + return naming.name, naming.content_type + + return self.name, self.content_type or "application/octet-stream" + @asynccontextmanager async def _open(self) -> AsyncGenerator[OpenedFileStream]: async with open_file_stream( @@ -150,6 +207,7 @@ async def _open(self) -> AsyncGenerator[OpenedFileStream]: prior_fetch_succeeded=self._prior_fetch_succeeded, client=self._client, credential=self._credential, + bot_api_credential=self._bot_api_credential, ) as opened: self._prior_fetch_succeeded = True yield opened @@ -160,4 +218,5 @@ def _target(self) -> FileFetchTarget: download_url=self._download_url, content_url=self.content_url, content_type=self.content_type, + attachment_style=self.attachment_style, ) diff --git a/packages/apps/src/microsoft_teams/apps/files/types.py b/packages/apps/src/microsoft_teams/apps/files/types.py index e4b79e97..9a79f729 100644 --- a/packages/apps/src/microsoft_teams/apps/files/types.py +++ b/packages/apps/src/microsoft_teams/apps/files/types.py @@ -11,3 +11,14 @@ - `botActivity` files come straight from the inbound activity's attachments; - `graph` files are hydrated through Microsoft Graph. """ + +AttachmentStyle = Literal["file", "inlineImage"] +""" +How a file was attached to the inbound message. +- `file`: sent as a file attachment +- `inlineImage`: pasted into the message body + +Some image formats, such as BMP and AVIF, cannot be converted by the platform, so Teams shows a placeholder, and +they cannot be downloaded. +The same image sent as a file attachment downloads normally. +""" diff --git a/packages/apps/tests/test_files_download_inline.py b/packages/apps/tests/test_files_download_inline.py new file mode 100644 index 00000000..8f65f08c --- /dev/null +++ b/packages/apps/tests/test_files_download_inline.py @@ -0,0 +1,396 @@ +""" +Copyright (c) Microsoft Corporation. All rights reserved. +Licensed under the MIT License. +""" + +# pyright: basic + +import json +from typing import List, Optional + +import httpx +import pytest +from microsoft_teams.api import ConversationType +from microsoft_teams.apps.files.download import ( + BotApiCredential, + FileFetchTarget, + is_under_service_url, + open_file_stream, +) +from microsoft_teams.apps.files.errors import ( + FileError, + InlineImageAccessError, + InlineImageTransformationError, +) +from microsoft_teams.apps.files.incoming_file import IncomingFile + +SERVICE_URL = "https://smba.trafficmanager.net/amer/" +CONTENT_URL = f"{SERVICE_URL}v3/attachments/a1/views/original" + +PNG = bytes([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A, 0x1A, 0x0A, 0, 0, 0, 0]) +GIF = bytes([0x47, 0x49, 0x46, 0x38, 0x39, 0x61, 0, 0]) +JPEG = bytes([0xFF, 0xD8, 0xFF, 0xE0, 0, 0]) +WEBP = bytes([0x52, 0x49, 0x46, 0x46, 0, 0, 0, 0, 0x57, 0x45, 0x42, 0x50]) +UNKNOWN = bytes([0x42, 0x4D, 0, 0, 0, 0]) + + +class _Recorder: + """Records every request the dispatcher makes, so a route's request count is observable rather than inferred.""" + + def __init__(self, status: int = 200, body: Optional[bytes] = None, content_type: Optional[str] = None) -> None: + self._status = status + self._body = body + self._content_type = content_type + self.calls: List[httpx.Request] = [] + + def handler(self, request: httpx.Request) -> httpx.Response: + self.calls.append(request) + headers = {"content-type": self._content_type} if self._content_type else {} + return httpx.Response(self._status, content=self._body or b"", headers=headers) + + @property + def client(self) -> httpx.AsyncClient: + return httpx.AsyncClient(transport=httpx.MockTransport(self.handler)) + + def auth_of(self, index: int) -> Optional[str]: + return self.calls[index].headers.get("Authorization") + + +class _CountingToken: + """A token source that reports whether it was ever resolved, so a refusal can be proven to precede it.""" + + def __init__(self, value: Optional[str] = "bot-api-token") -> None: + self._value = value + self.calls = 0 + + async def __call__(self) -> Optional[str]: + self.calls += 1 + return self._value + + +def _credential( + *, + actor: str = "app", + token: Optional[object] = None, + service_url: Optional[str] = SERVICE_URL, +) -> BotApiCredential: + return BotApiCredential(actor=actor, token=token or _CountingToken(), service_url=service_url) + + +def _inline_file( + recorder: _Recorder, + *, + name: str = "image-a1.gif", + content_type: Optional[str] = "image/gif", + scope: ConversationType = "groupChat", + credential: Optional[BotApiCredential] = None, + content_url: str = CONTENT_URL, +) -> IncomingFile: + return IncomingFile( + name=name, + content_type=content_type, + extension="gif", + scope=scope, + source="botActivity", + attachment_style="inlineImage", + content_url=content_url, + inline_image_base_name="image-a1", + bot_api_credential=credential or _credential(), + client=recorder.client, + ) + + +@pytest.mark.asyncio +async def test_reads_the_content_url_with_the_bot_api_token_in_a_scope_the_file_routes_do_not_open(): + recorder = _Recorder(body=PNG) + + await _inline_file(recorder, scope="channel").download() + + assert [str(call.url) for call in recorder.calls] == [CONTENT_URL] + assert recorder.auth_of(0) == "Bearer bot-api-token" + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "data, filename, content_type", + [ + (PNG, "image-a1.png", "image/png"), + (JPEG, "image-a1.jpg", "image/jpeg"), + (GIF, "image-a1.gif", "image/gif"), + (WEBP, "image-a1.webp", "image/webp"), + ], + ids=["PNG", "JPEG", "GIF", "WebP"], +) +async def test_names_and_types_bytes_from_the_bytes_over_the_hint_and_the_response_header( + data: bytes, filename: str, content_type: str +): + recorder = _Recorder(body=data, content_type="application/octet-stream") + + downloaded = await _inline_file(recorder, name="image-a1.gif", content_type="image/gif").download() + + assert downloaded.filename == filename + assert downloaded.content_type == content_type + + +@pytest.mark.asyncio +async def test_falls_back_to_the_hint_derived_name_and_type_when_the_bytes_are_not_recognized(): + recorder = _Recorder(body=UNKNOWN, content_type="image/png") + + downloaded = await _inline_file(recorder, name="image-a1.gif", content_type="image/gif").download() + + assert downloaded.filename == "image-a1.gif" + assert downloaded.content_type == "image/gif" + + +@pytest.mark.asyncio +async def test_falls_back_to_no_extension_and_octet_stream_when_neither_the_bytes_nor_the_hint_name_a_format(): + recorder = _Recorder(body=UNKNOWN, content_type="image/bmp") + + downloaded = await _inline_file(recorder, name="image-a1", content_type="application/octet-stream").download() + + assert downloaded.filename == "image-a1" + assert downloaded.content_type == "application/octet-stream" + + +@pytest.mark.asyncio +async def test_leaves_the_handle_unchanged_when_the_bytes_disagree_with_the_hint(): + recorder = _Recorder(body=PNG) + file = _inline_file(recorder, name="image-a1.gif", content_type="image/gif") + + await file.download() + + assert file.name == "image-a1.gif" + assert file.content_type == "image/gif" + assert file.extension == "gif" + + +@pytest.mark.asyncio +@pytest.mark.parametrize("value", [None, " "], ids=["missing", "blank"]) +async def test_throws_before_any_request_when_no_bot_api_token_is_available(value: Optional[str]): + recorder = _Recorder(body=PNG) + token = _CountingToken(value) + + with pytest.raises(RuntimeError, match="no Bot API token is available"): + await _inline_file(recorder, credential=_credential(token=token)).download() + + assert token.calls == 1 + assert recorder.calls == [] + + +@pytest.mark.asyncio +async def test_refuses_a_content_url_that_is_not_https(): + recorder = _Recorder(body=PNG) + + with pytest.raises(RuntimeError, match="must use https"): + async with open_file_stream( + FileFetchTarget( + scope="personal", + attachment_style="inlineImage", + content_url="http://smba.trafficmanager.net/amer/v3/attachments/a1/views/original", + ), + client=recorder.client, + bot_api_credential=_credential(), + ): + pass + + assert recorder.calls == [] + + +@pytest.mark.parametrize( + "url, service_url", + [ + (f"{SERVICE_URL}v3/attachments/a1/views/original", SERVICE_URL), + (f"{SERVICE_URL}v3/x", "https://smba.trafficmanager.net/amer"), + ("https://SMBA.trafficmanager.net/amer/v3/x", SERVICE_URL), + ("https://smba.trafficmanager.net/v3/x", "https://smba.trafficmanager.net"), + (f"{SERVICE_URL}%2e/v3/x", SERVICE_URL), + ("https://smba.trafficmanager.net/%2e/amer/v3/x", SERVICE_URL), + (f"{SERVICE_URL}..%2f..%2f/v3/x", SERVICE_URL), + ], + ids=[ + "the service URL path", + "a service URL without a trailing slash", + "a host that differs only in case", + "a service URL with no path", + "a percent-encoded single dot segment", + "a percent-encoded single dot segment ahead of the service URL path", + "an encoded slash, which is not a separator", + ], +) +def test_is_under_service_url_accepts(url: str, service_url: str): + assert is_under_service_url(url, service_url) is True + + +@pytest.mark.parametrize( + "url, service_url", + [ + ("https://smba.trafficmanager.net/americas/v3/x", "https://smba.trafficmanager.net/amer"), + (f"{SERVICE_URL}../v3/x", SERVICE_URL), + (f"{SERVICE_URL}%2e%2e/v3/x", SERVICE_URL), + (f"{SERVICE_URL}%2E%2e/v3/x", SERVICE_URL), + (f"{SERVICE_URL}.%2e/v3/x", SERVICE_URL), + ("https://attacker.example/amer/v3/x", SERVICE_URL), + ("https://smba.trafficmanager.net.attacker.example/amer/v3/x", "https://smba.trafficmanager.net"), + ("https://smba.trafficmanager.net:8443/amer/v3/x", SERVICE_URL), + ("http://smba.trafficmanager.net/amer/v3/x", SERVICE_URL), + ("http://smba.trafficmanager.net/amer/v3/x", "http://smba.trafficmanager.net/amer/"), + (f"{SERVICE_URL}v3/x", None), + (f"{SERVICE_URL}v3/x", "not a url"), + ("not a url", SERVICE_URL), + ], + ids=[ + "a sibling path that shares a prefix", + "a dot segment that climbs out", + "a percent-encoded dot segment that climbs out", + "a percent-encoded dot segment in mixed case", + "a half-encoded dot segment that climbs out", + "another host", + "a lookalike host", + "another port", + "an http target", + "an http service URL", + "a missing service URL", + "an unparseable service URL", + "an unparseable target", + ], +) +def test_is_under_service_url_rejects(url: str, service_url: Optional[str]): + assert is_under_service_url(url, service_url) is False + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "content_url", + [ + "https://attacker.example/amer/v3/attachments/a1/views/original", + f"{SERVICE_URL}../v3/attachments/a1/views/original", + ], + ids=["another host", "a dot segment that climbs out of the service URL path"], +) +async def test_refuses_a_forged_target_before_resolving_the_token_or_sending_a_request(content_url: str): + recorder = _Recorder(body=PNG) + token = _CountingToken() + + with pytest.raises(RuntimeError, match="content URL is not under the activity's service URL"): + await _inline_file(recorder, content_url=content_url, credential=_credential(token=token)).download() + + assert token.calls == 0 + assert recorder.calls == [] + + +@pytest.mark.asyncio +async def test_refuses_every_inline_image_when_the_credential_carries_no_service_url(): + recorder = _Recorder(body=PNG) + token = _CountingToken() + + with pytest.raises(RuntimeError, match="not under the activity's service URL"): + await _inline_file(recorder, credential=_credential(token=token, service_url=None)).download() + + assert token.calls == 0 + assert recorder.calls == [] + + +@pytest.mark.asyncio +async def test_resolves_a_plain_string_token_source(): + recorder = _Recorder(body=PNG) + + await _inline_file(recorder, credential=_credential(token="bot-api-token")).download() + + assert recorder.auth_of(0) == "Bearer bot-api-token" + + +TRANSFORMATION_BODY = json.dumps( + {"error": {"code": "TransformationFailed", "message": "Failed to transform the image"}} +).encode() + + +@pytest.mark.asyncio +async def test_maps_415_to_inline_image_transformation_error_with_the_status_code_and_service_text(): + recorder = _Recorder(status=415, body=TRANSFORMATION_BODY) + + with pytest.raises(InlineImageTransformationError) as caught: + await _inline_file(recorder).download() + + error = caught.value + assert isinstance(error, FileError) + assert error.status == 415 + assert error.code == "TransformationFailed" + assert error.details == "TransformationFailed: Failed to transform the image" + assert error.actor == "app" + assert str(error) == ( + "cannot download inline image: the platform could not convert it for download " + "(status 415, code TransformationFailed). Some image formats, such as BMP and AVIF, cannot be converted by " + "the platform, so Teams shows a placeholder, and they cannot be downloaded. Ask the user to send the image " + "as a file attachment instead (service said: TransformationFailed: Failed to transform the image)" + ) + + +@pytest.mark.asyncio +async def test_omits_the_code_segment_and_keeps_the_raw_text_when_a_415_body_is_not_json(): + recorder = _Recorder(status=415, body=b"unsupported") + + with pytest.raises(InlineImageTransformationError) as caught: + await _inline_file(recorder).download() + + error = caught.value + assert error.code is None + assert error.details == "unsupported" + assert "(status 415). Some image formats" in str(error) + assert "(service said: unsupported)" in str(error) + + +@pytest.mark.asyncio +async def test_omits_the_service_said_suffix_when_a_415_has_no_body(): + recorder = _Recorder(status=415) + + with pytest.raises(InlineImageTransformationError) as caught: + await _inline_file(recorder).download() + + error = caught.value + assert error.details is None + assert str(error).endswith("send the image as a file attachment instead") + + +@pytest.mark.asyncio +async def test_truncates_a_long_service_message(): + recorder = _Recorder(status=415, body=b"x" * 10_000) + + with pytest.raises(InlineImageTransformationError) as caught: + await _inline_file(recorder).download() + + assert caught.value.details == "x" * 2048 + + +@pytest.mark.asyncio +@pytest.mark.parametrize("status", [401, 403]) +async def test_maps_a_refusal_to_inline_image_access_error_naming_the_identity(status: int): + recorder = _Recorder( + status=status, body=json.dumps({"error": {"code": "Unauthorized", "message": "nope"}}).encode() + ) + agentic = _credential(actor="agentic_user", token="agent-token") + + with pytest.raises(InlineImageAccessError) as caught: + await _inline_file(recorder, credential=agentic).download() + + error = caught.value + assert isinstance(error, FileError) + assert error.status == status + assert error.actor == "agentic_user" + assert error.details == "Unauthorized: nope" + assert str(error) == ( + "cannot download inline image: the Bot API token presented for the agentic user was rejected with status " + f"{status} (service said: Unauthorized: nope)" + ) + + +@pytest.mark.asyncio +async def test_surfaces_another_failure_status_as_a_plain_error_carrying_the_service_text(): + recorder = _Recorder(status=500, body=json.dumps({"error": {"code": "ServerError", "message": "boom"}}).encode()) + + with pytest.raises(RuntimeError) as caught: + await _inline_file(recorder).download() + + error = caught.value + assert not isinstance(error, FileError) + assert "500" in str(error) + assert "ServerError: boom" in str(error) From 9721b194de7db32c9897ac9b78e4cee27e36f125 Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:32:29 -0700 Subject: [PATCH 03/11] List pasted inline images in ctx.files --- .../apps/files/files_accessor.py | 139 +++++++-- .../apps/tests/test_files_accessor_inline.py | 277 ++++++++++++++++++ 2 files changed, 394 insertions(+), 22 deletions(-) create mode 100644 packages/apps/tests/test_files_accessor_inline.py diff --git a/packages/apps/src/microsoft_teams/apps/files/files_accessor.py b/packages/apps/src/microsoft_teams/apps/files/files_accessor.py index e0733b55..5292cffa 100644 --- a/packages/apps/src/microsoft_teams/apps/files/files_accessor.py +++ b/packages/apps/src/microsoft_teams/apps/files/files_accessor.py @@ -17,28 +17,39 @@ ) from pydantic import ValidationError -from .download import GraphCredential, is_graph_route_open +from .download import BotApiCredential, GraphCredential, is_graph_route_open, is_under_service_url from .incoming_file import IncomingFile +from .inline_image import ( + format_from_content_type, + format_from_hint, + inline_image_id, + inline_image_naming, + parse_inline_image_hints, +) +from .types import AttachmentStyle logger = logging.getLogger(__name__) class FilesAccessor: """ - Accessor for the uploaded files on the current inbound activity, exposed as `ctx.files`. + Accessor for the uploaded files and inline images on the current inbound activity, exposed as `ctx.files`. + + "Files" is a filter over the raw `ctx.activity.attachments` array. Uploaded files arrive as attachments where + `content_type` is `file.download.info`, carrying file metadata rather than the bytes themselves. The metadata + names where the bytes live: a pre-authorized `download_url` when the platform issues one, otherwise a + `content_url` that locates the item so Graph can resolve it. What matters is how the content arrived, not the + file's MIME type, so any file type (pdf, docx, png, etc.) sent as an uploaded file is listed. - "Files" is the uploaded-file view over the raw `ctx.activity.attachments` array. Uploaded files arrive as - attachments where `content_type` is `file.download.info`, carrying file metadata rather than the bytes themselves. - The metadata names where the bytes live: a pre-authorized `download_url` when the platform issues one, otherwise a - `content_url` that locates the item so Graph can resolve it. This accessor maps each to an - `IncomingFile`, and skips everything else in `attachments` (adaptive cards, mentions, other non-file content) as - well as malformed file entries, never throwing. For each file it returns, the original wire attachment (the - metadata object, not the bytes) is retained on `IncomingFile.raw`. A malformed or non-file attachment is reachable - only through the raw `activity.attachments` array. + Images pasted inline into the message are included, with `IncomingFile.attachment_style` set to `inlineImage`. An + inline image not hosted on the activity's service URL is skipped with a warning. See `AttachmentStyle` for image + formats that cannot be downloaded. - This covers the file-upload path, not "any uploaded media". What matters is how the content arrived, not the - file's MIME type, so file *type* is unrestricted (pdf, docx, png, etc.) as long as it was sent as an uploaded - file. An image sent as a file appears here, but the same image pasted inline does not. + This accessor maps each listed attachment to an `IncomingFile`, and skips everything else in `attachments` + (adaptive cards, mentions, other non-file content) as well as malformed file entries, never throwing. For each + file it returns, the original wire attachment (the metadata object, not the bytes) is retained on + `IncomingFile.raw`. A malformed or non-file attachment is reachable only through the raw `activity.attachments` + array. The optional `client` is the app's shared `httpx.AsyncClient`, threaded into every `IncomingFile` so downloads reuse one connection pool instead of building and tearing down a client per file. It is the raw client rather @@ -51,20 +62,21 @@ def __init__( activity: ActivityBase, client: Optional[httpx.AsyncClient] = None, credential: Optional[GraphCredential] = None, + bot_api_credential: Optional[BotApiCredential] = None, ) -> None: self._activity = activity self._client = client self._credential = credential + self._bot_api_credential = bot_api_credential - async def list(self) -> list[IncomingFile]: + async def list(self, *, attachment_style: Optional[AttachmentStyle] = None) -> list[IncomingFile]: """ The files attached to the current inbound activity. Async because later scopes hydrate through Graph; the personal path resolves synchronously from the activity but keeps the async signature so the shape never breaks. - Currently takes no arguments and returns only uploaded files. The signature is reserved to grow options later - (e.g. `include_inline_images`, `content_types`, `include_raw`) so coverage can widen opt-in without a break; - the default stays narrow. + Returns uploaded files and inline images, and makes no network requests. Filter by attachment style using the + `attachment_style` option. """ # Uploaded files only ride on inbound message activities so we validate the shape and return an empty list # rather than throwing. @@ -73,21 +85,33 @@ async def list(self) -> list[IncomingFile]: attachments = self._activity.attachments or [] scope = self._detect_scope() + hints = self._inline_image_hints(attachments) files: list[IncomingFile] = [] for index, attachment in enumerate(attachments): - file = self._to_incoming_file(attachment, index, scope) + inline = self._is_inline_image(attachment) + + # Checked before mapping, so an attachment of the excluded style is never mapped and never logs a warning. + if attachment_style and attachment_style != ("inlineImage" if inline else "file"): + continue + + file = ( + self._to_inline_image(attachment, index, scope, hints) + if inline + else self._to_incoming_file(attachment, index, scope) + ) if file is not None: files.append(file) return files - async def first(self) -> Optional[IncomingFile]: + async def first(self, *, attachment_style: Optional[AttachmentStyle] = None) -> Optional[IncomingFile]: """ - Convenience: the first attached file, or `None` when none. Sugar over `list()[0]`; shares `list()`'s - resolution so it stays correct when later scopes hydrate through Graph. + Convenience: the first listed file or inline image, or `None` when none. Sugar over `list()[0]`; shares + `list()`'s resolution so it stays correct when later scopes hydrate through Graph. Filter by attachment style + using the `attachment_style` option. """ - files = await self.list() + files = await self.list(attachment_style=attachment_style) return files[0] if files else None def _detect_scope(self) -> ConversationType: @@ -96,6 +120,76 @@ def _detect_scope(self) -> ConversationType: conversation_type = getattr(conversation, "conversation_type", None) return conversation_type or "personal" + def _inline_image_hints(self, attachments: list[Attachment]) -> dict[str, str]: + """Format hints for inline images, keyed by attachment id, read from the message's HTML attachments.""" + hints: dict[str, str] = {} + for attachment in attachments: + if attachment.content_type == "text/html" and isinstance(attachment.content, str): + hints.update(parse_inline_image_hints(attachment.content)) + return hints + + def _is_inline_image(self, attachment: Attachment) -> bool: + """ + Whether an attachment is an image pasted inline: an `image/*` attachment located at a Bot API inline image + path. + """ + content_type = attachment.content_type + content_url = attachment.content_url + return ( + isinstance(content_type, str) + and content_type.lower().startswith("image/") + and isinstance(content_url, str) + and inline_image_id(content_url) is not None + ) + + def _to_inline_image( + self, + attachment: Attachment, + index: int, + scope: ConversationType, + hints: dict[str, str], + ) -> Optional[IncomingFile]: + """ + Map an inline image attachment to an `IncomingFile`, or `None` when its location is not on the activity's + service URL. + """ + content_url = cast(str, attachment.content_url) + id = cast(str, inline_image_id(content_url)) + + if not self._is_on_service_url(content_url): + logger.warning( + f"skipping inline image attachment at index {index}; its location is not on the activity's service URL" + ) + return None + + hint = hints.get(id) + format = format_from_hint(hint) if hint is not None else format_from_content_type(attachment.content_type) + base_name = f"image-{id}" + naming = inline_image_naming(base_name, format) + + return IncomingFile( + name=naming.name, + content_type=naming.content_type, + extension=naming.extension, + scope=scope, + source="botActivity", + attachment_style="inlineImage", + content_url=content_url, + raw=attachment, + inline_image_base_name=base_name, + client=self._client, + bot_api_credential=self._bot_api_credential, + ) + + def _is_on_service_url(self, url: str) -> bool: + """ + Whether a URL is under the activity's service URL over https. + + The download path applies the same check through `BotApiCredential.service_url`, while this one lets listing + skip and warn. + """ + return is_under_service_url(url, getattr(self._activity, "service_url", None)) + def _to_incoming_file(self, attachment: Attachment, index: int, scope: ConversationType) -> Optional[IncomingFile]: """ Map a single activity attachment to an `IncomingFile`, or `None` when the attachment is not an uploaded file @@ -142,6 +236,7 @@ def _to_incoming_file(self, attachment: Attachment, index: int, scope: Conversat name=name, scope=scope, source="botActivity", + attachment_style="file", unique_id=content.unique_id if content else None, # `file_type` is the platform-supplied extension (e.g. `pdf`); left `None` when the wire omits it, # matching how peer SDKs surface it. diff --git a/packages/apps/tests/test_files_accessor_inline.py b/packages/apps/tests/test_files_accessor_inline.py new file mode 100644 index 00000000..2ad6cfeb --- /dev/null +++ b/packages/apps/tests/test_files_accessor_inline.py @@ -0,0 +1,277 @@ +""" +Copyright (c) Microsoft Corporation. All rights reserved. +Licensed under the MIT License. +""" + +# pyright: basic + +import logging +from typing import List, Optional + +import pytest +from microsoft_teams.api import ( + FILE_DOWNLOAD_INFO_CONTENT_TYPE, + Account, + Attachment, + ConversationAccount, + MessageActivity, +) +from microsoft_teams.apps.files import FilesAccessor + +SERVICE_URL = "https://smba.trafficmanager.net/amer/" + + +def _inline_image(id: str, content_type: str = "image/*", service_url: str = SERVICE_URL) -> Attachment: + return Attachment(content_type=content_type, content_url=f"{service_url}v3/attachments/{id}/views/original") + + +def _html(*images: tuple[str, Optional[str]]) -> Attachment: + tags = "" + for id, scope in images: + itemscope = "" if scope is None else f' itemscope="{scope}"' + tags += ( + f'' + ) + return Attachment(content_type="text/html", content=f"

look

{tags}

") + + +def _activity_with(attachments: List[Attachment], service_url: Optional[str] = SERVICE_URL) -> MessageActivity: + activity = MessageActivity( + id="activity-id", + from_=Account(id="user-id"), + recipient=Account(id="bot-id"), + conversation=ConversationAccount(id="conversation-id", conversation_type="groupChat"), + attachments=attachments, + ) + activity.service_url = service_url + return activity + + +REPORT_PDF = Attachment( + content_type=FILE_DOWNLOAD_INFO_CONTENT_TYPE, + content_url="https://contoso.sharepoint.com/report.pdf", + name="report.pdf", + content={"downloadUrl": "https://download.example/report.pdf?tempauth=abc", "fileType": "pdf"}, +) + + +def _mixed() -> MessageActivity: + return _activity_with([_inline_image("a1"), REPORT_PDF, _inline_image("b2"), _html(("a1", "png"), ("b2", "png"))]) + + +@pytest.mark.parametrize( + "hint, name, extension, content_type", + [ + ("png", "image-a1.png", "png", "image/png"), + ("jpeg", "image-a1.jpg", "jpg", "image/jpeg"), + ("gif", "image-a1.gif", "gif", "image/gif"), + ("webp", "image-a1.webp", "webp", "image/webp"), + ], +) +async def test_names_and_types_a_hint(hint: str, name: str, extension: str, content_type: str) -> None: + image = _inline_image("a1") + + files = await FilesAccessor(_activity_with([image, _html((("a1"), hint))])).list() + + assert len(files) == 1 + file = files[0] + assert file.name == name + assert file.extension == extension + assert file.content_type == content_type + assert file.attachment_style == "inlineImage" + assert file.source == "botActivity" + assert file.scope == "groupChat" + assert file.content_url == image.content_url + assert file.raw is image + assert file.unique_id is None + + +@pytest.mark.parametrize("hint", ["bmp", "avif", ""]) +async def test_gives_an_unconvertible_hint_no_extension_and_octet_stream(hint: str) -> None: + files = await FilesAccessor(_activity_with([_inline_image("a1"), _html(("a1", hint))])).list() + + assert files[0].name == "image-a1" + assert files[0].extension is None + assert files[0].content_type == "application/octet-stream" + + +async def test_gives_a_missing_hint_no_extension_and_octet_stream_rather_than_assuming_png() -> None: + files = await FilesAccessor(_activity_with([_inline_image("a1"), _html(("a1", None))])).list() + + assert files[0].name == "image-a1" + assert files[0].content_type == "application/octet-stream" + + +async def test_falls_back_to_the_attachment_content_type_only_when_no_hint_is_present() -> None: + without_html = await FilesAccessor(_activity_with([_inline_image("a1", "image/jpeg")])).list() + wildcard = await FilesAccessor(_activity_with([_inline_image("a1", "image/*")])).list() + hint_wins = await FilesAccessor(_activity_with([_inline_image("a1", "image/jpeg"), _html(("a1", "bmp"))])).list() + + assert (without_html[0].name, without_html[0].content_type) == ("image-a1.jpg", "image/jpeg") + assert (wildcard[0].name, wildcard[0].content_type) == ("image-a1", "application/octet-stream") + assert (hint_wins[0].name, hint_wins[0].content_type) == ("image-a1", "application/octet-stream") + + +async def test_lists_several_inline_images_alongside_a_file_attachment_each_with_its_own_style() -> None: + file = Attachment( + content_type=FILE_DOWNLOAD_INFO_CONTENT_TYPE, + content_url="https://contoso.sharepoint.com/report.pdf", + name="report.pdf", + content={ + "downloadUrl": "https://download.example/report.pdf?tempauth=abc", + "fileType": "pdf", + "uniqueId": "u1", + }, + ) + activity = _activity_with([_inline_image("a1"), file, _inline_image("b2"), _html(("a1", "png"), ("b2", "webp"))]) + + files = await FilesAccessor(activity).list() + + assert [(f.name, f.attachment_style) for f in files] == [ + ("image-a1.png", "inlineImage"), + ("report.pdf", "file"), + ("image-b2.webp", "inlineImage"), + ] + assert files[1].unique_id == "u1" + + +async def test_returns_both_styles_when_the_attachment_style_option_is_omitted() -> None: + files = await FilesAccessor(_mixed()).list() + + assert [f.name for f in files] == ["image-a1.png", "report.pdf", "image-b2.png"] + + +async def test_returns_only_file_attachments_for_file() -> None: + files = await FilesAccessor(_mixed()).list(attachment_style="file") + + assert [f.name for f in files] == ["report.pdf"] + + +async def test_returns_only_pasted_images_for_inline_image() -> None: + files = await FilesAccessor(_mixed()).list(attachment_style="inlineImage") + + assert [f.name for f in files] == ["image-a1.png", "image-b2.png"] + + +async def test_lets_first_skip_a_leading_pasted_image() -> None: + accessor = FilesAccessor(_mixed()) + + first = await accessor.first() + first_file = await accessor.first(attachment_style="file") + + assert first is not None and first.name == "image-a1.png" + assert first_file is not None and first_file.name == "report.pdf" + + +async def test_lets_first_return_none_when_no_file_has_the_style() -> None: + accessor = FilesAccessor(_activity_with([_inline_image("a1")])) + + assert await accessor.first(attachment_style="file") is None + + +async def test_does_not_warn_about_an_inline_image_it_was_never_going_to_return( + caplog: pytest.LogCaptureFixture, +) -> None: + elsewhere = _inline_image("a1", "image/*", "https://attacker.example/amer/") + + with caplog.at_level(logging.WARNING): + files = await FilesAccessor(_activity_with([elsewhere, REPORT_PDF])).list(attachment_style="file") + + assert [f.name for f in files] == ["report.pdf"] + assert caplog.records == [] + + +async def test_reads_hints_regardless_of_attribute_order_or_quote_style() -> None: + reordered = Attachment( + content_type="text/html", + content="\"x\"", + ) + + files = await FilesAccessor(_activity_with([_inline_image("a1"), reordered])).list() + + assert files[0].name == "image-a1.gif" + + +async def test_ignores_an_img_hint_that_is_not_marked_as_an_inline_image() -> None: + other = Attachment(content_type="text/html", content='') + + files = await FilesAccessor(_activity_with([_inline_image("a1"), other])).list() + + assert files[0].name == "image-a1" + + +async def test_skips_an_inline_image_that_is_not_on_the_activity_service_url_with_a_warning( + caplog: pytest.LogCaptureFixture, +) -> None: + elsewhere = _inline_image("a1", "image/*", "https://attacker.example/amer/") + + with caplog.at_level(logging.WARNING): + files = await FilesAccessor(_activity_with([elsewhere, _inline_image("b2")])).list() + + assert [f.name for f in files] == ["image-b2"] + assert "skipping inline image attachment at index 0; its location is not on the activity's service URL" in ( + caplog.text + ) + + +async def test_does_not_let_a_lookalike_host_match_a_service_url_that_lacks_a_trailing_slash( + caplog: pytest.LogCaptureFixture, +) -> None: + lookalike = _inline_image("a1", "image/*", "https://smba.trafficmanager.net.attacker.example/") + + with caplog.at_level(logging.WARNING): + files = await FilesAccessor(_activity_with([lookalike], "https://smba.trafficmanager.net")).list() + + assert files == [] + assert "not on the activity's service URL" in caplog.text + + +async def test_does_not_let_a_dot_segment_climb_out_of_the_service_url_path( + caplog: pytest.LogCaptureFixture, +) -> None: + climbing = _inline_image("a1", "image/*", "https://smba.trafficmanager.net/amer/TENANT/../../") + + with caplog.at_level(logging.WARNING): + files = await FilesAccessor(_activity_with([climbing], "https://smba.trafficmanager.net/amer/TENANT/")).list() + + assert files == [] + assert "not on the activity's service URL" in caplog.text + + +async def test_matches_a_service_url_without_a_trailing_slash() -> None: + files = await FilesAccessor( + _activity_with( + [_inline_image("a1", "image/*", "https://smba.trafficmanager.net/amer/")], + "https://smba.trafficmanager.net/amer", + ) + ).list() + + assert len(files) == 1 + + +async def test_matches_a_service_url_whose_host_differs_only_in_case_keeping_the_content_url_as_sent() -> None: + image = _inline_image("a1", "image/*", "https://SMBA.TrafficManager.net/amer/") + + files = await FilesAccessor(_activity_with([image], "https://smba.trafficmanager.net/amer/")).list() + + assert len(files) == 1 + assert files[0].content_url == image.content_url + + +async def test_skips_inline_images_when_the_activity_has_no_service_url( + caplog: pytest.LogCaptureFixture, +) -> None: + with caplog.at_level(logging.WARNING): + files = await FilesAccessor(_activity_with([_inline_image("a1")], None)).list() + + assert files == [] + assert "not on the activity's service URL" in caplog.text + + +async def test_leaves_an_image_attachment_that_is_not_at_an_inline_image_location_alone() -> None: + attachment = Attachment(content_type="image/png", content_url=f"{SERVICE_URL}v3/somewhere/else.png") + + files = await FilesAccessor(_activity_with([attachment])).list() + + assert files == [] From 8bb8b9e37d2133c01eeda1870ab640ac33f8d5f6 Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:34:13 -0700 Subject: [PATCH 04/11] Use the turn's Bot API token for inline images --- .../src/microsoft_teams/apps/app_process.py | 10 ++- .../microsoft_teams/apps/files_credential.py | 31 +++++++- .../apps/routing/activity_context.py | 13 +++- packages/apps/tests/test_app_process.py | 78 +++++++++++++++++++ packages/apps/tests/test_files_credential.py | 68 +++++++++++++++- 5 files changed, 193 insertions(+), 7 deletions(-) diff --git a/packages/apps/src/microsoft_teams/apps/app_process.py b/packages/apps/src/microsoft_teams/apps/app_process.py index 0cd6891c..1759af77 100644 --- a/packages/apps/src/microsoft_teams/apps/app_process.py +++ b/packages/apps/src/microsoft_teams/apps/app_process.py @@ -44,7 +44,7 @@ record_turn_duration, ) from .events import ActivityEvent, ActivityResponseEvent, ActivitySentEvent, ErrorEvent -from .files_credential import select_files_credential +from .files_credential import select_files_credential, select_inline_image_credential from .oauth_flow import OAuthFlowRegistry from .plugins import PluginActivityEvent, PluginBase, StreamCancelledError from .routing.activity_context import ActivityContext @@ -172,6 +172,13 @@ async def _build_context( get_agentic_graph_token=lambda identity: self.get_agentic_graph_token(identity, tenant_id), ) + # Inline images are read with the turn's Bot API token, not Graph. + files_bot_api_credential = select_inline_image_credential( + agentic_identity=activity.recipient.agentic_identity, + service_url=activity.service_url, + get_token=lambda: api_client.http.token, + ) + activityCtx = ActivityContext( activity, self.id or "", @@ -185,6 +192,7 @@ async def _build_context( cloud=self.cloud, oauth_connection_names=list(self.oauth_registry) if self.oauth_registry is not None else None, files_credential=files_credential, + files_bot_api_credential=files_bot_api_credential, ) send = activityCtx.send diff --git a/packages/apps/src/microsoft_teams/apps/files_credential.py b/packages/apps/src/microsoft_teams/apps/files_credential.py index 7766bfc8..5ed18abb 100644 --- a/packages/apps/src/microsoft_teams/apps/files_credential.py +++ b/packages/apps/src/microsoft_teams/apps/files_credential.py @@ -7,8 +7,9 @@ from typing import Optional from microsoft_teams.api import AgenticIdentity, TokenProtocol +from microsoft_teams.common.http.client_token import Token, resolve_token -from .files.download import GraphCredential +from .files.download import BotApiCredential, GraphCredential from .files.errors import FileActor @@ -47,3 +48,31 @@ async def app_token() -> Optional[str]: return str(token) if token else None return _as("app", app_token) + + +def select_inline_image_credential( + *, + agentic_identity: Optional[AgenticIdentity], + service_url: Optional[str], + get_token: Callable[[], Token], +) -> BotApiCredential: + """ + Build the credential that reads inline images for the current activity, using the turn's Bot API token. + + `service_url` is the activity's service URL; the token is only sent to URLs under it. + `get_token` returns the API client's token source: a string, a string-like value, or a factory. + + The token is resolved lazily, so a turn that never downloads an inline image never acquires one. + """ + + async def token() -> Optional[str]: + resolved = await resolve_token(get_token()) + # `resolve_token` passes a blank value through, and a blank bearer token is no token at all. + value = resolved.strip() if resolved else None + return value or None + + return BotApiCredential( + actor="agentic_user" if agentic_identity is not None else "app", + service_url=service_url, + token=token, + ) diff --git a/packages/apps/src/microsoft_teams/apps/routing/activity_context.py b/packages/apps/src/microsoft_teams/apps/routing/activity_context.py index 565b6034..8f244544 100644 --- a/packages/apps/src/microsoft_teams/apps/routing/activity_context.py +++ b/packages/apps/src/microsoft_teams/apps/routing/activity_context.py @@ -62,7 +62,7 @@ ) from ..diagnostics._helpers import trace_oauth_operation from ..files import FilesAccessor -from ..files.download import GraphCredential +from ..files.download import BotApiCredential, GraphCredential from ..http_stream import HttpStream from ..oauth_connection import connection_lookup_key, normalize_connection_name from ..oauth_state import ( @@ -131,6 +131,7 @@ def __init__( cloud: CloudEnvironment = PUBLIC, oauth_connection_names: Optional[Sequence[str]] = None, files_credential: Optional[GraphCredential] = None, + files_bot_api_credential: Optional[BotApiCredential] = None, ): self.activity = activity self.app_id = app_id @@ -151,6 +152,7 @@ def __init__( self._stream: Optional[StreamerProtocol] = None self._files: Optional[FilesAccessor] = None self._files_credential = files_credential + self._files_bot_api_credential = files_bot_api_credential self._next_handler: Optional[Callable[[], Awaitable[None]]] = None @@ -168,13 +170,18 @@ def stream(self) -> StreamerProtocol: def files(self) -> FilesAccessor: """ The uploaded files on the current inbound activity, i.e. the `content_type: file.download.info` subset of - `activity.attachments`, mapped to `IncomingFile`. See `FilesAccessor`. + `activity.attachments`, plus images pasted inline, mapped to `IncomingFile`. See `FilesAccessor`. """ if self._files is None: # Reuse the API client's underlying connection pool rather than building a new one per download. The raw # `httpx.AsyncClient` is used deliberately: the SDK wrapper injects the bot's `Authorization` header per # request, and a download URL carries its own `tempauth` credential that a bearer token can displace. - self._files = FilesAccessor(self.activity, self.api.http.http, self._files_credential) + self._files = FilesAccessor( + self.activity, + self.api.http.http, + self._files_credential, + self._files_bot_api_credential, + ) return self._files @property diff --git a/packages/apps/tests/test_app_process.py b/packages/apps/tests/test_app_process.py index 143334ba..3c5bf57c 100644 --- a/packages/apps/tests/test_app_process.py +++ b/packages/apps/tests/test_app_process.py @@ -824,6 +824,84 @@ async def test_build_context_gives_ctx_files_an_app_credential_without_agentic_i assert credential is not None assert credential.actor == "app" + @pytest.mark.asyncio + async def test_build_context_reads_inline_images_with_the_token_of_the_turns_own_api_client( + self, activity_processor + ): + """An inbound Agentic User activity must reach `ctx.files` as an agentic Bot API credential.""" + activity_processor.fetch_user_token = False + activity_processor.http_client.http = MagicMock() + core_activity = CoreActivity( + type="message", + id="activity-inline-credential", + service_url="https://service.url/", + **{ + "from": {"id": "user-1", "name": "Test User"}, + "conversation": {"id": "conv-1"}, + "recipient": { + "id": "bot-1", + "name": "Test Bot", + "agenticAppId": "agentic-app-id", + "agenticUserId": "agentic-user-id", + "agenticAppBlueprintId": "blueprint-id", + "tenantId": "tenant-id", + }, + "channelId": "msteams", + }, + ) + activity = ActivityTypeAdapter.validate_python(core_activity.model_dump(by_alias=True, exclude_none=True)) + mock_token = MagicMock(spec=TokenProtocol) + mock_token.service_url = "https://service.url/" + + context = await activity_processor._build_context(activity, mock_token, []) + credential = context.files._bot_api_credential + + assert credential is not None + assert credential.actor == "agentic_user" + + # The client's token is scoped to the inbound identity, so an agentic identity reads as itself. + context.api.http.token = "turn-client-token" + assert await credential.token() == "turn-client-token" + + @pytest.mark.asyncio + async def test_build_context_resolves_the_turn_clients_token_factory_and_carries_the_service_url( + self, activity_processor + ): + """The credential reads the API client's token source at fetch time, factory shape included.""" + activity_processor.fetch_user_token = False + activity_processor.http_client.http = MagicMock() + core_activity = CoreActivity( + type="message", + id="activity-inline-factory", + service_url="https://service.url/amer/", + **{ + "from": {"id": "user-1", "name": "Test User"}, + "conversation": {"id": "conv-1"}, + "recipient": {"id": "bot-1", "name": "Test Bot"}, + "channelId": "msteams", + }, + ) + activity = ActivityTypeAdapter.validate_python(core_activity.model_dump(by_alias=True, exclude_none=True)) + mock_token = MagicMock(spec=TokenProtocol) + mock_token.service_url = "https://service.url/amer/" + + context = await activity_processor._build_context(activity, mock_token, []) + credential = context.files._bot_api_credential + + assert credential is not None + assert credential.actor == "app" + assert credential.service_url == "https://service.url/amer/" + + calls: list[int] = [] + + async def factory() -> str: + calls.append(1) + return "factory-token" + + context.api.http.token = factory + assert await credential.token() == "factory-token" + assert calls == [1] + @pytest.mark.asyncio async def test_build_context_skips_token_fetch_when_disabled(self, activity_processor): """When fetch_user_token is False, the user-token API is not called.""" diff --git a/packages/apps/tests/test_files_credential.py b/packages/apps/tests/test_files_credential.py index 0d8aa204..bd00f134 100644 --- a/packages/apps/tests/test_files_credential.py +++ b/packages/apps/tests/test_files_credential.py @@ -6,11 +6,11 @@ # pyright: basic from typing import Optional -from unittest.mock import AsyncMock +from unittest.mock import AsyncMock, Mock import pytest from microsoft_teams.api import AgenticIdentity -from microsoft_teams.apps.files_credential import select_files_credential +from microsoft_teams.apps.files_credential import select_files_credential, select_inline_image_credential class FakeToken: @@ -120,3 +120,67 @@ async def test_reports_no_token_rather_than_raising_when_the_app_has_no_credenti credential, _, _ = _selector() assert await credential.token() is None + + +class TestSelectInlineImageCredential: + @pytest.mark.asyncio + async def test_reads_as_the_app_without_an_agentic_identity_and_as_the_agentic_user_with_one(self): + as_app = select_inline_image_credential(agentic_identity=None, service_url=None, get_token=lambda: "bot-token") + as_agentic = select_inline_image_credential( + agentic_identity=AGENTIC, service_url=None, get_token=lambda: "bot-token" + ) + + assert as_app.actor == "app" + assert as_agentic.actor == "agentic_user" + + @pytest.mark.asyncio + async def test_resolves_a_string_a_string_like_value_and_a_token_factory(self): + async def factory() -> str: + return "factory" + + from_string = select_inline_image_credential(agentic_identity=None, service_url=None, get_token=lambda: "plain") + from_string_like = select_inline_image_credential( + agentic_identity=None, service_url=None, get_token=lambda: FakeToken("string-like") + ) + from_factory = select_inline_image_credential( + agentic_identity=None, service_url=None, get_token=lambda: factory + ) + + assert await from_string.token() == "plain" + assert await from_string_like.token() == "string-like" + assert await from_factory.token() == "factory" + + @pytest.mark.asyncio + @pytest.mark.parametrize("source", [None, " ", ""]) + async def test_reports_no_token_for_an_absent_or_blank_source(self, source: Optional[str]): + credential = select_inline_image_credential(agentic_identity=None, service_url=None, get_token=lambda: source) + + assert await credential.token() is None + + @pytest.mark.asyncio + async def test_carries_the_service_url_it_was_given_as_the_allowed_destination(self): + credential = select_inline_image_credential( + agentic_identity=None, + service_url="https://smba.trafficmanager.net/amer/", + get_token=lambda: "bot-token", + ) + + assert credential.service_url == "https://smba.trafficmanager.net/amer/" + + @pytest.mark.asyncio + async def test_reads_the_token_source_at_fetch_time_rather_than_when_built(self): + current = "first" + credential = select_inline_image_credential(agentic_identity=None, service_url=None, get_token=lambda: current) + + current = "second" + + assert await credential.token() == "second" + + @pytest.mark.asyncio + async def test_acquires_no_token_until_one_is_asked_for(self): + # A turn that never downloads an inline image should never pay for a token. + get_token = Mock(return_value="bot-token") + + select_inline_image_credential(agentic_identity=None, service_url=None, get_token=get_token) + + get_token.assert_not_called() From 35a918323bddd8b8c3792fdb9404c7b8155a16f1 Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:41:42 -0700 Subject: [PATCH 05/11] Show pasted images in ai-file-analysis --- examples/ai-file-analysis/README.md | 3 ++- examples/ai-file-analysis/src/file_card.py | 1 + examples/ai-file-analysis/src/main.py | 14 ++++++++++---- 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/examples/ai-file-analysis/README.md b/examples/ai-file-analysis/README.md index ddfb138b..7868c2d3 100644 --- a/examples/ai-file-analysis/README.md +++ b/examples/ai-file-analysis/README.md @@ -58,7 +58,7 @@ uv run src/main.py ## What happens to an attached file -1. `ctx.files.list()` returns the files on the incoming activity. +1. `ctx.files.list()` returns the files on the incoming activity. Images pasted into the message are listed alongside attached files, with `attachment_style` set to `"inlineImage"`. To filter by type, use filter options, e.g. `ctx.files.list(attachment_style="file")`. 2. Each file is downloaded once and reused from memory, rather than refetched through the short-lived URL. 3. `classify_file` sorts each download into `text`, `image`, or `unsupported`. 4. Unsupported files get the basic Adaptive Card. No model call is made for them. @@ -72,6 +72,7 @@ A file arrives as metadata, not as bytes. Which route turns that metadata into b - **A bot reads through the pre-authorized download URL.** Teams provides a URL on the attachment and the SDK fetches it directly, with no tokens attached, shown in this example. - **An agentic user reads through Microsoft Graph.** Agentic users never receive a pre-authorized URL, so the SDK resolves the file through Graph's `/shares` endpoint using the agent's own credential. +- **A pasted image reads through the Bot API.** The SDK downloads it with the same token the app uses to reply in that conversation, not Graph. Some image formats, such as BMP and AVIF, cannot be converted by the platform, so Teams shows a placeholder, and they cannot be downloaded. For those, `download()` raises `InlineImageTransformationError`. Teams saves a pasted GIF as a static PNG, so it is listed as `.gif` and `image/gif` but downloads as a PNG. ### The download URL expires, and that is final diff --git a/examples/ai-file-analysis/src/file_card.py b/examples/ai-file-analysis/src/file_card.py index 6b6757bf..6c32da21 100644 --- a/examples/ai-file-analysis/src/file_card.py +++ b/examples/ai-file-analysis/src/file_card.py @@ -39,6 +39,7 @@ def unsupported_file_card(file: IncomingFile, downloaded: DownloadedFile, note: Fact(title="Size", value=_human_size(len(downloaded.bytes))), Fact(title="Scope", value=file.scope), Fact(title="Listed from", value=file.source), + Fact(title="Attachment style", value=file.attachment_style), ] ), TextBlock( diff --git a/examples/ai-file-analysis/src/main.py b/examples/ai-file-analysis/src/main.py index a4a7a835..c05ba447 100644 --- a/examples/ai-file-analysis/src/main.py +++ b/examples/ai-file-analysis/src/main.py @@ -18,7 +18,7 @@ from ai import AnalyzableFile, classify_file, is_ai_configured, prepare_analysis, run_analysis from file_card import unsupported_file_card from microsoft_teams.api import MessageActivity, TypingActivityInput -from microsoft_teams.apps import ActivityContext, App +from microsoft_teams.apps import ActivityContext, App, InlineImageTransformationError logging.basicConfig(level=logging.INFO) logger = logging.getLogger("ai-file-analysis") @@ -50,9 +50,10 @@ async def handle_message(ctx: ActivityContext[MessageActivity]) -> None: attached = await ctx.files.list() if not attached: await ctx.send( - "Attach one or more files. I analyze text files and images, and describe anything else I cannot read." + "Attach or paste one or more files. I analyze text files and images, and describe anything else I " + "cannot read." if _ai_configured - else "Attach one or more files. No model is configured, so I will report what I received " + else "Attach or paste one or more files. No model is configured, so I will report what I received " "without analyzing it." ) return @@ -66,7 +67,12 @@ async def handle_message(ctx: ActivityContext[MessageActivity]) -> None: downloaded = await file.download() except Exception as err: logger.warning("Could not download %s: %s", file.name, err) - await ctx.send(f"I could not download {file.name}.") + await ctx.send( + "I could not download that pasted image because its format cannot be converted. " + "Try sending it as a file attachment instead." + if isinstance(err, InlineImageTransformationError) + else f"I could not download {file.name}." + ) continue if not _ai_configured: From aa44b11392422b6d39e320a2ec56cb3251ba827b Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Mon, 12 Oct 2026 13:45:52 -0700 Subject: [PATCH 06/11] Authorization is dropped on any cross-host redirect unit tests --- .../apps/tests/test_files_download_graph.py | 37 ++++++++++++++ .../apps/tests/test_files_download_inline.py | 48 +++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/packages/apps/tests/test_files_download_graph.py b/packages/apps/tests/test_files_download_graph.py index 73a60d42..405b3eca 100644 --- a/packages/apps/tests/test_files_download_graph.py +++ b/packages/apps/tests/test_files_download_graph.py @@ -511,6 +511,43 @@ async def test_does_not_forward_the_bearer_to_the_storage_host(self): assert rec.auth_of(0) == f"Bearer {AGENTIC_JWT}" assert rec.auth_of(1) is None + @pytest.mark.asyncio + @pytest.mark.parametrize( + "label,location", + [ + ("a subdomain of the original host", "https://files.graph.microsoft.com/blob/1"), + ("another port on the original host", "https://graph.microsoft.com:8443/blob/1"), + ], + ) + async def test_drops_the_bearer_on_a_redirect_to(self, label: str, location: str): + # A subdomain and a port change are both a different origin, and neither is as obviously foreign as a + # SharePoint host. Node's redirect follower keeps `Authorization` on a subdomain hop, so TypeScript strips + # it by hand; httpx drops it on any host or port change, and this pins that rather than trusting it. + rec = _RedirectRecorder(location) + + async with open_file_stream( + _target(content_url=CONTENT_URL), client=rec.client(), credential=_credential("agentic_user", AGENTIC_JWT) + ): + pass + + assert len(rec.calls) == 2 + assert rec.auth_of(1) is None + + @pytest.mark.asyncio + async def test_drops_a_lowercase_authorization_default_on_a_cross_origin_redirect(self): + # A caller's own default header can arrive in any casing, so the drop cannot be keyed on the exact spelling + # the SDK happens to write. + rec = _RedirectRecorder("https://files.graph.microsoft.com/blob/1") + client = rec.client(headers={"authorization": "Bearer caller-default"}) + + async with open_file_stream( + _target(content_url=CONTENT_URL), client=client, credential=_credential("agentic_user", AGENTIC_JWT) + ): + pass + + assert len(rec.calls) == 2 + assert rec.auth_of(1) is None + @pytest.mark.asyncio async def test_the_preauth_path_follows_a_redirect(self): # httpx defaults follow_redirects to False, so the pre-authorized path has to opt in explicitly. TypeScript diff --git a/packages/apps/tests/test_files_download_inline.py b/packages/apps/tests/test_files_download_inline.py index 8f65f08c..7eaa3d4e 100644 --- a/packages/apps/tests/test_files_download_inline.py +++ b/packages/apps/tests/test_files_download_inline.py @@ -394,3 +394,51 @@ async def test_surfaces_another_failure_status_as_a_plain_error_carrying_the_ser assert not isinstance(error, FileError) assert "500" in str(error) assert "ServerError: boom" in str(error) + + +class _InlineRedirectRecorder(_Recorder): + """Serves a 302 to `location` on the first call and image bytes on the second, recording both hops.""" + + def __init__(self, location: str) -> None: + super().__init__() + self._location = location + + def handler(self, request: httpx.Request) -> httpx.Response: + self.calls.append(request) + if len(self.calls) == 1: + return httpx.Response(302, headers={"location": self._location}) + return httpx.Response(200, content=PNG) + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "label,location", + [ + ("a subdomain of the original host", "https://files.smba.trafficmanager.net/blob/1"), + ("another port on the original host", "https://smba.trafficmanager.net:8443/blob/1"), + ("an unrelated host", "https://storage.example/blob/1"), + ], +) +async def test_drops_the_bot_api_token_on_a_redirect_to(label: str, location: str): + # The Bot API token is the app's own credential for that conversation, so it must not reach the host the bytes + # happen to live on, however close that host looks to the service URL. + recorder = _InlineRedirectRecorder(location) + + downloaded = await _inline_file(recorder).download() + + assert downloaded.bytes == PNG + assert len(recorder.calls) == 2 + assert recorder.auth_of(0) is not None + assert recorder.auth_of(1) is None + + +@pytest.mark.asyncio +async def test_keeps_the_bot_api_token_on_a_redirect_that_stays_on_the_original_host(): + # `test_drops_the_bot_api_token_on_a_redirect_to` would also pass if the header were dropped unconditionally. + # This pins that the drop is conditional on the origin changing. + recorder = _InlineRedirectRecorder(f"{SERVICE_URL}v3/attachments/a1/views/original/2") + + await _inline_file(recorder).download() + + assert len(recorder.calls) == 2 + assert recorder.auth_of(1) == recorder.auth_of(0) From 54a266e713bbedd12db77d91120b14b28f55ac32 Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:32:35 -0700 Subject: [PATCH 07/11] fix(apps): annotate inline image hint attachments as Sequence Inside FilesAccessor, `list` resolves to the accessor's own `list()` method, so Pyright could not type the parameter. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../apps/src/microsoft_teams/apps/files/files_accessor.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/apps/src/microsoft_teams/apps/files/files_accessor.py b/packages/apps/src/microsoft_teams/apps/files/files_accessor.py index 5292cffa..5737ec06 100644 --- a/packages/apps/src/microsoft_teams/apps/files/files_accessor.py +++ b/packages/apps/src/microsoft_teams/apps/files/files_accessor.py @@ -4,7 +4,7 @@ """ import logging -from typing import Any, Optional, cast +from typing import Any, Optional, Sequence, cast import httpx from microsoft_teams.api import ( @@ -120,7 +120,7 @@ def _detect_scope(self) -> ConversationType: conversation_type = getattr(conversation, "conversation_type", None) return conversation_type or "personal" - def _inline_image_hints(self, attachments: list[Attachment]) -> dict[str, str]: + def _inline_image_hints(self, attachments: Sequence[Attachment]) -> dict[str, str]: """Format hints for inline images, keyed by attachment id, read from the message's HTML attachments.""" hints: dict[str, str] = {} for attachment in attachments: From 88b85e8455d4c4649d95d33352efa6135bde9fa5 Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:43:54 -0700 Subject: [PATCH 08/11] fix(apps): harden inline image URL and tag matching Compare service URL paths case-sensitively, match img tags in linear time with quote awareness, and skip inline image hint parsing when listing only file attachments. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/microsoft_teams/apps/files/download.py | 14 ++++++++++++-- .../microsoft_teams/apps/files/files_accessor.py | 2 +- .../src/microsoft_teams/apps/files/inline_image.py | 2 +- packages/apps/tests/test_files_download_inline.py | 4 ++++ packages/apps/tests/test_inline_image.py | 14 ++++++++++++++ 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/packages/apps/src/microsoft_teams/apps/files/download.py b/packages/apps/src/microsoft_teams/apps/files/download.py index 69772a18..b30eb763 100644 --- a/packages/apps/src/microsoft_teams/apps/files/download.py +++ b/packages/apps/src/microsoft_teams/apps/files/download.py @@ -7,6 +7,7 @@ import binascii import json import logging +import re from collections.abc import AsyncGenerator, AsyncIterator, Awaitable, Callable from contextlib import asynccontextmanager from dataclasses import dataclass @@ -112,6 +113,8 @@ class BotApiCredential: _SINGLE_DOT_SEGMENTS = frozenset({".", "%2e"}) _DOUBLE_DOT_SEGMENTS = frozenset({"..", ".%2e", "%2e.", "%2e%2e"}) +_PERCENT_ESCAPE = re.compile(r"%[0-9A-Fa-f]{2}") + def _resolve_dot_segments(path: str) -> str: """ @@ -143,6 +146,8 @@ def is_under_service_url(url: str, service_url: Optional[str]) -> bool: path. Both are parsed first, and their dot segments are resolved, so a path cannot climb out of the service URL's path. + The scheme and host compare case-insensitively. + The path compares case-sensitively, apart from the hex digits of percent-escapes, which mean the same in any case. """ if not service_url: return False @@ -162,8 +167,13 @@ def is_under_service_url(url: str, service_url: Optional[str]) -> bool: return False # Normalizes to exactly one trailing `/` so a path such as `/amer` cannot match `/americas`. - base_path = _resolve_dot_segments(base.path).rstrip("/") + "/" - return _resolve_dot_segments(target.path).lower().startswith(base_path.lower()) + base_path = _normalize_percent_escapes(_resolve_dot_segments(base.path).rstrip("/") + "/") + return _normalize_percent_escapes(_resolve_dot_segments(target.path)).startswith(base_path) + + +def _normalize_percent_escapes(path: str) -> str: + """Uppercase the hex digits of each percent-escape, leaving every other character as it is.""" + return _PERCENT_ESCAPE.sub(lambda match: match.group(0).upper(), path) @dataclass diff --git a/packages/apps/src/microsoft_teams/apps/files/files_accessor.py b/packages/apps/src/microsoft_teams/apps/files/files_accessor.py index 5737ec06..79739230 100644 --- a/packages/apps/src/microsoft_teams/apps/files/files_accessor.py +++ b/packages/apps/src/microsoft_teams/apps/files/files_accessor.py @@ -85,7 +85,7 @@ async def list(self, *, attachment_style: Optional[AttachmentStyle] = None) -> l attachments = self._activity.attachments or [] scope = self._detect_scope() - hints = self._inline_image_hints(attachments) + hints = self._inline_image_hints(attachments) if attachment_style != "file" else {} files: list[IncomingFile] = [] for index, attachment in enumerate(attachments): diff --git a/packages/apps/src/microsoft_teams/apps/files/inline_image.py b/packages/apps/src/microsoft_teams/apps/files/inline_image.py index 280fb787..13ce9248 100644 --- a/packages/apps/src/microsoft_teams/apps/files/inline_image.py +++ b/packages/apps/src/microsoft_teams/apps/files/inline_image.py @@ -36,7 +36,7 @@ separators out of it. """ -_IMG_TAG = re.compile(r"]*>", re.IGNORECASE | re.ASCII) +_IMG_TAG = re.compile(r"\"']|\"[^\"]*\"|'[^']*')*>", re.IGNORECASE | re.ASCII) _ATTRIBUTE = re.compile(r"(? None: + started = time.monotonic() + + for shape in (" None: + html = f'1 > 0' + + assert parse_inline_image_hints(html) == {"a": "png"} + + def test_hints_match_tag_and_attribute_names_in_any_case() -> None: assert parse_inline_image_hints(f'') == {"a": "png"} From 90e29837b8ef2d682c66249c147abc1fe3987a18 Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:34:40 -0700 Subject: [PATCH 09/11] Update references to 'platform' to 'Teams service' --- examples/ai-file-analysis/README.md | 2 +- packages/apps/src/microsoft_teams/apps/files/errors.py | 8 ++++---- packages/apps/src/microsoft_teams/apps/files/types.py | 2 +- packages/apps/tests/test_files_download_inline.py | 7 ++++--- 4 files changed, 10 insertions(+), 9 deletions(-) diff --git a/examples/ai-file-analysis/README.md b/examples/ai-file-analysis/README.md index 7868c2d3..acc41b1c 100644 --- a/examples/ai-file-analysis/README.md +++ b/examples/ai-file-analysis/README.md @@ -72,7 +72,7 @@ A file arrives as metadata, not as bytes. Which route turns that metadata into b - **A bot reads through the pre-authorized download URL.** Teams provides a URL on the attachment and the SDK fetches it directly, with no tokens attached, shown in this example. - **An agentic user reads through Microsoft Graph.** Agentic users never receive a pre-authorized URL, so the SDK resolves the file through Graph's `/shares` endpoint using the agent's own credential. -- **A pasted image reads through the Bot API.** The SDK downloads it with the same token the app uses to reply in that conversation, not Graph. Some image formats, such as BMP and AVIF, cannot be converted by the platform, so Teams shows a placeholder, and they cannot be downloaded. For those, `download()` raises `InlineImageTransformationError`. Teams saves a pasted GIF as a static PNG, so it is listed as `.gif` and `image/gif` but downloads as a PNG. +- **A pasted image reads through the Bot API.** The SDK downloads it with the same token the app uses to reply in that conversation, not Graph. Some image formats, such as BMP and AVIF, cannot be converted by the Teams service, so Teams shows a placeholder, and they cannot be downloaded. For those, `download()` raises `InlineImageTransformationError`. Teams saves a pasted GIF as a static PNG, so it is listed as `.gif` and `image/gif` but downloads as a PNG. ### The download URL expires, and that is final diff --git a/packages/apps/src/microsoft_teams/apps/files/errors.py b/packages/apps/src/microsoft_teams/apps/files/errors.py index 4c95a005..951d078e 100644 --- a/packages/apps/src/microsoft_teams/apps/files/errors.py +++ b/packages/apps/src/microsoft_teams/apps/files/errors.py @@ -155,7 +155,7 @@ def __init__( class InlineImageTransformationError(FileError): """ - Raised when the platform could not convert an inline image for download, answering `415`. + Raised when the Teams service could not convert an inline image for download, answering `415`. The same image sent as a file attachment downloads normally. """ @@ -181,9 +181,9 @@ def __init__( ) -> None: reason = f"status {status}, code {code}" if code else f"status {status}" message = ( - f"cannot download inline image: the platform could not convert it for download ({reason}). Some image " - "formats, such as BMP and AVIF, cannot be converted by the platform, so Teams shows a placeholder, and " - "they cannot be downloaded. Ask the user to send the image as a file attachment instead" + f"cannot download inline image: the Teams service could not convert it for download ({reason}). " + "Some image formats, such as BMP and AVIF, cannot be converted by the Teams service, so Teams shows a " + "placeholder, and they cannot be downloaded. Ask the user to send the image as a file attachment instead" ) if details: message = f"{message} (service said: {details})" diff --git a/packages/apps/src/microsoft_teams/apps/files/types.py b/packages/apps/src/microsoft_teams/apps/files/types.py index 9a79f729..a9d442ee 100644 --- a/packages/apps/src/microsoft_teams/apps/files/types.py +++ b/packages/apps/src/microsoft_teams/apps/files/types.py @@ -18,7 +18,7 @@ - `file`: sent as a file attachment - `inlineImage`: pasted into the message body -Some image formats, such as BMP and AVIF, cannot be converted by the platform, so Teams shows a placeholder, and +Some image formats, such as BMP and AVIF, cannot be converted by the Teams service, so Teams shows a placeholder, and they cannot be downloaded. The same image sent as a file attachment downloads normally. """ diff --git a/packages/apps/tests/test_files_download_inline.py b/packages/apps/tests/test_files_download_inline.py index 7a84e7a8..ae9ee288 100644 --- a/packages/apps/tests/test_files_download_inline.py +++ b/packages/apps/tests/test_files_download_inline.py @@ -322,10 +322,11 @@ async def test_maps_415_to_inline_image_transformation_error_with_the_status_cod assert error.details == "TransformationFailed: Failed to transform the image" assert error.actor == "app" assert str(error) == ( - "cannot download inline image: the platform could not convert it for download " + "cannot download inline image: the Teams service could not convert it for download " "(status 415, code TransformationFailed). Some image formats, such as BMP and AVIF, cannot be converted by " - "the platform, so Teams shows a placeholder, and they cannot be downloaded. Ask the user to send the image " - "as a file attachment instead (service said: TransformationFailed: Failed to transform the image)" + "the Teams service, so Teams shows a placeholder, and they cannot be downloaded. " + "Ask the user to send the image as a file attachment instead " + "(service said: TransformationFailed: Failed to transform the image)" ) From f47d5bcb6aeba8fd6d0a279e75260b78b0d8077c Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:35:01 -0700 Subject: [PATCH 10/11] Apply feedback --- .../microsoft_teams/apps/files/download.py | 2 +- .../apps/files/files_accessor.py | 29 +++++++++---------- .../apps/files/incoming_file.py | 4 +-- .../apps/files/inline_image.py | 15 ++++------ 4 files changed, 23 insertions(+), 27 deletions(-) diff --git a/packages/apps/src/microsoft_teams/apps/files/download.py b/packages/apps/src/microsoft_teams/apps/files/download.py index b30eb763..96a255b3 100644 --- a/packages/apps/src/microsoft_teams/apps/files/download.py +++ b/packages/apps/src/microsoft_teams/apps/files/download.py @@ -55,7 +55,7 @@ class FileFetchTarget: content_type: Optional[str] = None """MIME type reported by the incoming file, used as a fallback when the response omits one.""" - attachment_style: Optional[AttachmentStyle] = None + attachment_style: AttachmentStyle = "file" """ How the file was attached; see `AttachmentStyle`. An `inlineImage` is read from `content_url` with the Bot API credential, in any scope. diff --git a/packages/apps/src/microsoft_teams/apps/files/files_accessor.py b/packages/apps/src/microsoft_teams/apps/files/files_accessor.py index 79739230..4665d0ef 100644 --- a/packages/apps/src/microsoft_teams/apps/files/files_accessor.py +++ b/packages/apps/src/microsoft_teams/apps/files/files_accessor.py @@ -89,16 +89,16 @@ async def list(self, *, attachment_style: Optional[AttachmentStyle] = None) -> l files: list[IncomingFile] = [] for index, attachment in enumerate(attachments): - inline = self._is_inline_image(attachment) + image_id = self._inline_image_id(attachment) # Checked before mapping, so an attachment of the excluded style is never mapped and never logs a warning. - if attachment_style and attachment_style != ("inlineImage" if inline else "file"): + if attachment_style and attachment_style != ("file" if image_id is None else "inlineImage"): continue file = ( - self._to_inline_image(attachment, index, scope, hints) - if inline - else self._to_incoming_file(attachment, index, scope) + self._to_incoming_file(attachment, index, scope) + if image_id is None + else self._to_inline_image(attachment, image_id, index, scope, hints) ) if file is not None: files.append(file) @@ -128,23 +128,23 @@ def _inline_image_hints(self, attachments: Sequence[Attachment]) -> dict[str, st hints.update(parse_inline_image_hints(attachment.content)) return hints - def _is_inline_image(self, attachment: Attachment) -> bool: + def _inline_image_id(self, attachment: Attachment) -> Optional[str]: """ - Whether an attachment is an image pasted inline: an `image/*` attachment located at a Bot API inline image - path. + The inline image id when an attachment is an image pasted inline (an `image/*` attachment located at a Bot API + inline image path), otherwise `None`. """ content_type = attachment.content_type content_url = attachment.content_url - return ( - isinstance(content_type, str) - and content_type.lower().startswith("image/") - and isinstance(content_url, str) - and inline_image_id(content_url) is not None - ) + if not isinstance(content_type, str) or not content_type.lower().startswith("image/"): + return None + if not isinstance(content_url, str): + return None + return inline_image_id(content_url) def _to_inline_image( self, attachment: Attachment, + id: str, index: int, scope: ConversationType, hints: dict[str, str], @@ -154,7 +154,6 @@ def _to_inline_image( service URL. """ content_url = cast(str, attachment.content_url) - id = cast(str, inline_image_id(content_url)) if not self._is_on_service_url(content_url): logger.warning( diff --git a/packages/apps/src/microsoft_teams/apps/files/incoming_file.py b/packages/apps/src/microsoft_teams/apps/files/incoming_file.py index 39bfc2b1..1160b802 100644 --- a/packages/apps/src/microsoft_teams/apps/files/incoming_file.py +++ b/packages/apps/src/microsoft_teams/apps/files/incoming_file.py @@ -109,7 +109,7 @@ def __init__( unique_id: Optional[str] = None, content_type: Optional[str] = None, extension: Optional[str] = None, - attachment_style: Optional[AttachmentStyle] = None, + attachment_style: AttachmentStyle = "file", content_url: Optional[str] = None, raw: Any = None, inline_image_base_name: Optional[str] = None, @@ -124,7 +124,7 @@ def __init__( self.unique_id = unique_id self.content_type = content_type self.extension = extension - self.attachment_style = attachment_style or "file" + self.attachment_style = attachment_style self.content_url = content_url self.raw = raw self._inline_image_base_name = inline_image_base_name diff --git a/packages/apps/src/microsoft_teams/apps/files/inline_image.py b/packages/apps/src/microsoft_teams/apps/files/inline_image.py index 13ce9248..f6ef3afb 100644 --- a/packages/apps/src/microsoft_teams/apps/files/inline_image.py +++ b/packages/apps/src/microsoft_teams/apps/files/inline_image.py @@ -127,24 +127,21 @@ def sniff_image_format(data: bytes) -> Optional[InlineImageFormat]: The format the bytes actually are, read from their leading magic bytes, or `None` when they are none of `InlineImageFormat`. - The platform can return bytes in a different format from the hint (a GIF downloads as a static PNG), so the bytes - are the stronger signal. + The Teams service can return bytes in a different format from the hint (a GIF downloads as a static PNG), so the + bytes are the stronger signal. """ - def starts_with(signature: bytes, offset: int = 0) -> bool: - return len(data) >= offset + len(signature) and data[offset : offset + len(signature)] == signature - - if starts_with(b"\x89PNG\r\n\x1a\n"): + if data.startswith(b"\x89PNG\r\n\x1a\n"): return "png" - if starts_with(b"\xff\xd8\xff"): + if data.startswith(b"\xff\xd8\xff"): return "jpeg" - if starts_with(b"GIF87a") or starts_with(b"GIF89a"): + if data.startswith((b"GIF87a", b"GIF89a")): return "gif" # "RIFF" at byte 0 and "WEBP" at byte 8; bytes 4-7 are the file size. - if starts_with(b"RIFF") and starts_with(b"WEBP", 8): + if data.startswith(b"RIFF") and data.startswith(b"WEBP", 8): return "webp" return None From 947c877dfd3fdf76adc50d1a86405ec947c2ec88 Mon Sep 17 00:00:00 2001 From: Corina <14900841+corinagum@users.noreply.github.com> Date: Wed, 14 Oct 2026 09:30:46 -0700 Subject: [PATCH 11/11] Clarify inline image download errors --- .../microsoft_teams/apps/files/download.py | 16 +++++---- .../src/microsoft_teams/apps/files/errors.py | 26 ++++++++++---- .../apps/tests/test_files_download_inline.py | 36 ++++++++++++++++--- 3 files changed, 61 insertions(+), 17 deletions(-) diff --git a/packages/apps/src/microsoft_teams/apps/files/download.py b/packages/apps/src/microsoft_teams/apps/files/download.py index 96a255b3..31592388 100644 --- a/packages/apps/src/microsoft_teams/apps/files/download.py +++ b/packages/apps/src/microsoft_teams/apps/files/download.py @@ -432,11 +432,14 @@ async def _open_inline_image_stream( if not url.lower().startswith("https://"): raise RuntimeError("cannot download inline image: content URL must use https") - if not is_under_service_url(url, credential.service_url if credential else None): + if credential is None: + raise RuntimeError("cannot download inline image: no Bot API token is available") + + if not is_under_service_url(url, credential.service_url): raise RuntimeError("cannot download inline image: content URL is not under the activity's service URL") - actor = credential.actor if credential else None - token = (await resolve_token(credential.token) if credential else None) or "" + actor = credential.actor + token = (await resolve_token(credential.token)) or "" # `resolve_token` passes a blank value through, and a blank bearer token is no token at all. if not token.strip(): @@ -456,9 +459,9 @@ async def _open_inline_image_stream( body = await _read_error_body(response) raise InlineImageTransformationError( response.status_code, - _extract_service_error_code(body), - _extract_service_error(body), actor, + _extract_service_error(body), + _extract_service_error_code(body), ) if response.status_code in (401, 403): @@ -467,8 +470,7 @@ async def _open_inline_image_stream( if not response.is_success: details = await _read_service_error(response) message = ( - f"failed to download inline image as '{actor or 'app'}': " - f"{response.status_code} {response.reason_phrase}" + f"failed to download inline image as '{actor}': {response.status_code} {response.reason_phrase}" ).strip() raise RuntimeError(f"{message} ({details})" if details else message) diff --git a/packages/apps/src/microsoft_teams/apps/files/errors.py b/packages/apps/src/microsoft_teams/apps/files/errors.py index 951d078e..237fffda 100644 --- a/packages/apps/src/microsoft_teams/apps/files/errors.py +++ b/packages/apps/src/microsoft_teams/apps/files/errors.py @@ -175,9 +175,9 @@ class InlineImageTransformationError(FileError): def __init__( self, status: int, - code: Optional[str] = None, - details: Optional[str] = None, actor: Optional[FileActor] = None, + details: Optional[str] = None, + code: Optional[str] = None, ) -> None: reason = f"status {status}, code {code}" if code else f"status {status}" message = ( @@ -217,10 +217,7 @@ def __init__( actor: Optional[FileActor] = None, details: Optional[str] = None, ) -> None: - as_who = _describe_actor(actor) if actor else "the identity used" - message = ( - f"cannot download inline image: the Bot API token presented for {as_who} was rejected with status {status}" - ) + message = f"cannot download inline image: {_inline_access_guidance(status, actor)}" if details: message = f"{message} (service said: {details})" super().__init__(message) @@ -229,6 +226,23 @@ def __init__( self.details = details +def _inline_access_guidance(status: int, actor: Optional[FileActor]) -> str: + """ + Say what an inline image refusal means, which depends on the status. + + A rejected token and a denied identity have different remedies. + """ + as_who = _describe_actor(actor) if actor else "the identity used" + if status == 401: + return ( + f"the Bot API token presented for {as_who} was rejected (status 401). " + "It may have expired, or been issued for the wrong audience or tenant" + ) + if status == 403: + return f"access was denied for {as_who} (status 403). The identity is not allowed to read the image" + return f"the Bot API token presented for {as_who} was rejected with status {status}" + + def _describe_actor(actor: FileActor) -> str: """ Name the identity in prose. Exhaustive on purpose: a new `FileActor` must fail type checking here rather than diff --git a/packages/apps/tests/test_files_download_inline.py b/packages/apps/tests/test_files_download_inline.py index ae9ee288..1c626b3d 100644 --- a/packages/apps/tests/test_files_download_inline.py +++ b/packages/apps/tests/test_files_download_inline.py @@ -177,6 +177,27 @@ async def test_throws_before_any_request_when_no_bot_api_token_is_available(valu assert recorder.calls == [] +@pytest.mark.asyncio +async def test_reports_the_missing_token_not_the_service_url_when_there_is_no_bot_api_credential(): + recorder = _Recorder(body=PNG) + file = IncomingFile( + name="image-a1.gif", + content_type="image/gif", + extension="gif", + scope="groupChat", + source="botActivity", + attachment_style="inlineImage", + content_url=CONTENT_URL, + inline_image_base_name="image-a1", + client=recorder.client, + ) + + with pytest.raises(RuntimeError, match="cannot download inline image: no Bot API token is available"): + await file.download() + + assert recorder.calls == [] + + @pytest.mark.asyncio async def test_refuses_a_content_url_that_is_not_https(): recorder = _Recorder(body=PNG) @@ -382,10 +403,17 @@ async def test_maps_a_refusal_to_inline_image_access_error_naming_the_identity(s assert error.status == status assert error.actor == "agentic_user" assert error.details == "Unauthorized: nope" - assert str(error) == ( - "cannot download inline image: the Bot API token presented for the agentic user was rejected with status " - f"{status} (service said: Unauthorized: nope)" - ) + expected = { + 401: ( + "cannot download inline image: the Bot API token presented for the agentic user was rejected (status 401). " + "It may have expired, or been issued for the wrong audience or tenant (service said: Unauthorized: nope)" + ), + 403: ( + "cannot download inline image: access was denied for the agentic user (status 403). " + "The identity is not allowed to read the image (service said: Unauthorized: nope)" + ), + } + assert str(error) == expected[status] @pytest.mark.asyncio