Port AttachmentDownloader and M365AttachmentDownloader from .NET - #574
Closed
Rodrigo Brandão (rodrigobr-msft) wants to merge 16 commits into
Closed
Rodrigo Brandão (rodrigobr-msft) wants to merge 16 commits into
Rodrigo Brandão (rodrigobr-msft) wants to merge 16 commits into
Conversation
Rodrigo Brandão (rodrigobr-msft)
linked an issue
Copilot started reviewing on behalf of
Rodrigo Brandão (rodrigobr-msft)
September 10, 2026 13:28
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved security and functional issues remain in attachment downloading and authentication.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds generic and M365 attachment downloading, attachment URI construction, MIME parsing, claims handling, and a sample application.
Changes:
- Adds authenticated and anonymous attachment downloaders.
- Extends attachment models, channels, and audience-claim handling.
- Adds connector URI APIs, tests, and handling-attachments sample configuration.
File summaries
| File | Summary |
|---|---|
tests/hosting_core/connector/test_connector_client.py |
Tests attachment URI construction. |
tests/hosting_core/app/test_m365_attachment_downloader.py |
Tests M365 attachment downloading. |
tests/hosting_core/app/test_attachment_downloader.py |
Tests generic attachment downloading. |
tests/hosting_core/app/streaming/test_streaming_response.py |
Updates streaming response formatting. |
test_samples/handling_attachments/src/main.py |
Adds the sample server entry point. |
test_samples/handling_attachments/src/agent.py |
Adds attachment-handling sample behavior. |
test_samples/handling_attachments/src/__init__.py |
Marks the sample source as a package. |
test_samples/handling_attachments/requirements.txt |
Defines sample dependencies. |
test_samples/handling_attachments/README.md |
Provides sample documentation. |
test_samples/handling_attachments/env.TEMPLATE |
Provides sample configuration. |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/mcs/mcs_connector_client.py |
Adds unsupported MCS attachment URI handling. |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/client/connector_client.py |
Builds encoded attachment URIs. |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/connector/attachments_base.py |
Extends the attachment connector interface. |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/claims_identity.py |
Adds audience-claim helpers. |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py |
Implements M365 attachment downloading. |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/input_file.py |
Adds optional input-file metadata. |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py |
Implements generic attachment downloading. |
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/_utils.py |
Parses content-type headers. |
libraries/microsoft-agents-activity/microsoft_agents/activity/channels.py |
Adds the M365 Copilot channel. |
libraries/microsoft-agents-activity/microsoft_agents/activity/attachment.py |
Allows optional attachment URL and name. |
Review details
Suppressed comments (11)
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/_utils.py:23
- MIME types are case-insensitive, but this preserves the header's casing. A valid header such as
IMAGE/JPEGthen fails the downloaders'content_type.startswith("image/")check and is not normalized like lowercase image types; case-normalize the main type before returning it.
return params[0][0], dict(params[1:])
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py:22
- These new public downloader classes are not re-exported from
microsoft_agents.hosting.core.appormicrosoft_agents.hosting.core, unlikeInputFileandInputFileDownloader. Consumers configuringApplicationOptions.file_downloadersmust use module-level imports instead of the package API; export both downloaders and add them to the relevant__all__lists.
class AttachmentDownloader(InputFileDownloader):
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:105
ConnectionManager.get_token_provider_from_activity()raisesValueErrorwhen no mapping/default connection exists; it does not returnNone. Consequently the intendedRuntimeErrorbelow is bypassed for the real connection manager, unlike this test double. Catch that lookup error and normalize it to the existing no-provider path.
if not token_provider:
token_provider = self._connections.get_token_provider_from_activity(
context.identity, context.activity
)
if not token_provider:
raise RuntimeError("No valid token provider found.")
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:68
AgentApplicationinvokes every configured file downloader for every turn, so a non-M365 activity can reach this method with no identity. The current ordering raises before the channel-specific downloader can return[], aborting otherwise valid non-M365 turns. Move the channel filter before identity and audience resolution.
if not context.identity:
raise ValueError("No valid context identity found.")
outgoing_audience_claim = context.identity.get_outgoing_audience_claim()
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:103
- The named-provider branch is not actually verified here:
_FakeConnections.get_connection()andget_token_provider_from_activity()both return the same provider, so this test passes even iftoken_provider_nameis ignored. Use distinct providers and assert that only the named provider was requested.
if self._token_provider_name:
try:
token_provider = self._connections.get_connection(
self._token_provider_name
)
except ValueError:
pass
if not token_provider:
token_provider = self._connections.get_token_provider_from_activity(
context.identity, context.activity
)
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/claims_identity.py:4
- The new
reimport is unused anywhere in this module and will trigger the repository's unused-import lint warning; remove it unless the audience logic is meant to use it.
import re
test_samples/handling_attachments/src/agent.py:141
- This second data URI has the same bytes-to-string problem:
b64encode()is interpolated as a bytes repr (b'...') rather than base64 text. Decode it before formatting the attachment URL.
image_data = base64.b64encode(image_path.read_bytes())
test_samples/handling_attachments/src/agent.py:155
Path(__file__)again points at the Python file rather than its containing directory, so the upload path resolves to a nonexistentsrc/agent.py/resources/...location and the upload option always fails. Resolve the repository parent before appendingresources.
image_path = Path(__file__) / "resources" / "agents-sdk.png"
test_samples/handling_attachments/src/agent.py:58
- The three string literals are concatenated without spaces, producing user-visible text such as
Agent.Thisandattachments.Please. Include trailing spaces or use adjacent literals with spaces between the sentences.
"Welcome to the HandlingAttachments Agent." +
"This agent will introduce you to attachments." +
"Please select an option."
tests/hosting_core/app/test_attachment_downloader.py:170
- The test name says the downloader returns a
Noneentry, butdownload_files()filters failed downloads and the assertion expects an empty list. Rename the test to describe the observable behavior, such as omitting failed downloads.
async def test_returns_none_for_non_success_status(self):
tests/hosting_core/app/test_m365_attachment_downloader.py:303
- This test name says the result contains a
Noneentry, but the implementation filters that entry and the assertion expects[]. Rename it to describe that failed downloads are omitted.
async def test_returns_none_entry_for_failed_status(self):
- Files reviewed: 18/23 changed files
- Comments generated: 11
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+63
to
+66
| if attachment.content_url and ( | ||
| attachment.content_url.startswith("https://") | ||
| or attachment.content_url.startswith("http://localhost") | ||
| ): |
Comment on lines
+130
to
+133
| if attachment.content_url and ( | ||
| attachment.content_url.startswith("https://") | ||
| or attachment.content_url.startswith("http://localhost") | ||
| ): |
Comment on lines
+135
to
+145
| if isinstance(attachment.content, dict): | ||
| content_dict = cast(dict[str, Any], attachment.content) | ||
| download_url = content_dict.get("downloadUrl", attachment.content_url) | ||
| else: | ||
| download_url = attachment.content_url | ||
|
|
||
| if ( | ||
| self._host_validator is not None | ||
| and self._host_validator.enabled | ||
| and not self._host_validator.is_allowed(download_url) | ||
| ): |
Comment on lines
+19
to
+20
| if params is None: | ||
| return None |
Comment on lines
+72
to
+76
| if context.activity.channel_id not in ( | ||
| Channels.ms_teams, | ||
| Channels.m365_copilot, | ||
| ): | ||
| return [] |
Comment on lines
+115
to
+118
| if self.is_agent_claim(): | ||
| return f"api://{self.get_outgoing_app_id()}" | ||
| if self.is_gov_botframework_claim(): | ||
| return AuthenticationConstants.GOV_AGENTS_SDK_TOKEN_ISSUER |
| return None | ||
| # the first param is the mime-type | ||
| # the later ones are the attribtues like "charset" | ||
| return params[0][0], dict(params[1:]) |
Copilot started reviewing on behalf of
Rodrigo Brandão (rodrigobr-msft)
September 10, 2026 15:23
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved moderate issues remain in downloader validation, authentication handling, content processing, and the sample.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (16)
libraries/microsoft-agents-activity/microsoft_agents/activity/attachment.py:26
- The field annotations now allow
content_urlandnameto beNone, but the class docstring still documents both fields asstr. Update the type entries so the public documentation matches the new nullable contract.
content_url: NonEmptyString | None = None
content: object = None
name: NonEmptyString | None = None
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/_utils.py:23
Message.get_params()can return an empty list for an emptyContent-Typeheader; this test only handlesNone, soparams[0]raisesIndexErrorinstead of returningNone. A response withoutContent-Typewill therefore crash both downloaders rather than being skipped.
params = email.get_params()
if params is None:
return None
# the first param is the mime-type
# the later ones are the attribtues like "charset"
return params[0][0], dict(params[1:])
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/_utils.py:22
- Correct the typo in this new comment.
# the later ones are the attribtues like "charset"
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py:66
startswith("http://localhost")also accepts hosts such aslocalhost.evil.example, so the downloader's local-only HTTP exception is not actually limited to localhost when host validation is disabled. Parse the URL and compare its hostname exactly tolocalhost(or reuse the shared validator) before issuing the request.
if attachment.content_url and (
attachment.content_url.startswith("https://")
or attachment.content_url.startswith("http://localhost")
):
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py:77
- A new
ClientSessionis created and closed for every remote attachment. For a message containing multiple files this disables connection pooling and adds session/connector setup overhead per file; create one session for thedownload_filesoperation and reuse it across the loop.
async with self._client_factory() as client:
async with client.get(remote_file_url) as response:
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py:53
- The inline branch serializes every attachment without a URL, including well-known card attachments such as
application/vnd.microsoft.card.heroand adaptive cards. Since AgentApplication invokes configured file downloaders for every turn, configuring this downloader causes cards to appear instate.temp.input_filesas JSON files. Exclude card/control attachment content types before treating inline content as a file.
files: list[InputFile] = []
for attachment in context.activity.attachments:
file = await self._download_file(attachment)
if file:
files.append(file)
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:150
- When
use_anonymous=True,access_tokenremains empty, but this still sendsAuthorization: Bearer. Some servers reject that malformed credential, so anonymous mode is not actually anonymous. Only add the Authorization header when a token was acquired.
async with client.get(
download_url, headers={"Authorization": f"Bearer {access_token}"}
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:134
- The URL/scheme check is applied only to
attachment.content_url; adownloadUrlinattachment.contentreplaces it and is then used without the same validation. With the optional host validator disabled, a crafted attachment can redirect this token-bearing request to an arbitrary HTTP or internal endpoint. Validate the finaldownload_urlwith URL parsing (including an exactlocalhosthostname) before calling the client.
if attachment.content_url and (
attachment.content_url.startswith("https://")
or attachment.content_url.startswith("http://localhost")
):
download_url: str
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:70
- These checks run before the channel filter and before the
use_anonymousbranch. As a result, an identity-less non-M365 turn fails instead of returning an empty list, anduse_anonymous=Truestill requires an identity and an outgoing audience even though no token is needed. Move the channel check first and only require the identity/audience for authenticated downloads.
if not context.identity:
raise ValueError("No valid context identity found.")
outgoing_audience_claim = context.identity.get_outgoing_audience_claim()
if not outgoing_audience_claim:
raise ValueError("No valid outgoing App ID found.")
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:133
- The same prefix-based localhost check accepts attacker-controlled hosts such as
localhost.evil.example, and this downloader sends an Authorization header to the resulting request. Use parsed host validation for the actual download URL rather than a string prefix.
if attachment.content_url and (
attachment.content_url.startswith("https://")
or attachment.content_url.startswith("http://localhost")
):
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:151
- A new
ClientSessionis created and closed for every remote attachment. Multiple M365 attachments therefore lose connection pooling and incur repeated connector setup; reuse one session for the wholedownload_filescall.
async with self._client_factory() as client:
async with client.get(
download_url, headers={"Authorization": f"Bearer {access_token}"}
) as response:
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:85
- Filtering only
text/htmldoes not prevent card attachments from reaching this inline branch. A Teams/M365 card with a JSONcontentobject will be serialized intoInputFileand exposed as a user file, so exclude known card/control content types before downloading files.
attachments = [
att
for att in context.activity.attachments
if not att.content_type.startswith("text/html")
]
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/claims_identity.py:119
- Government Bot Framework identities can satisfy
is_agent_claim()because that method rejects only the public issuer. As a result, this ordering returnsapi://<appid>beforeis_gov_botframework_claim()is checked, making the new government audience branch ineffective for versioned claims withappid/azp. Check the government issuer first.
if self.is_agent_claim():
return f"api://{self.get_outgoing_app_id()}"
if self.is_gov_botframework_claim():
return AuthenticationConstants.GOV_AGENTS_SDK_TOKEN_ISSUER
return AuthenticationConstants.AGENTS_SDK_SCOPE
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/claims_identity.py:119
- The new audience-selection branches are not covered by
test_claims_identity.py, and the M365 downloader test only checks that a provider was called rather than the resource it received. Add cases for the default, government-cloud, and agent-claim paths (including token versions) and assert the requested audience so a regression here cannot silently acquire the wrong token.
def get_outgoing_audience_claim(self) -> str | None:
"""Retrieves the audience for an outgoing token from the given incoming activity."""
if self.is_agent_claim():
return f"api://{self.get_outgoing_app_id()}"
if self.is_gov_botframework_claim():
return AuthenticationConstants.GOV_AGENTS_SDK_TOKEN_ISSUER
return AuthenticationConstants.AGENTS_SDK_SCOPE
test_samples/handling_attachments/src/agent.py:66
- This handler is registered for
/helpas well asmembersAdded. Message activities normally havemembers_added=None, so/helpraisesTypeErrorbefore it can display the attachment options. Handle the absent list and send the help response before iterating.
async def _help(context: TurnContext, _state: TurnState):
for member in context.activity.members_added:
test_samples/handling_attachments/src/agent.py:71
- The concatenated welcome message has no spaces after
Agent.orattachments., so the sample displaysAgent.Thisandattachments.Please. Keep the separators in the user-facing text.
"Welcome to the HandlingAttachments Agent." +
"This agent will introduce you to attachments." +
"Please select an option."
- Files reviewed: 19/23 changed files
- Comments generated: 2
- Review effort level: Lite
Comment on lines
+65
to
+76
| if not context.identity: | ||
| raise ValueError("No valid context identity found.") | ||
|
|
||
| outgoing_audience_claim = context.identity.get_outgoing_audience_claim() | ||
| if not outgoing_audience_claim: | ||
| raise ValueError("No valid outgoing App ID found.") | ||
|
|
||
| if context.activity.channel_id not in ( | ||
| Channels.ms_teams, | ||
| Channels.m365_copilot, | ||
| ): | ||
| return [] |
Comment on lines
+126
to
+129
| name=state.temp.input_files[0].filename, | ||
| content_type="image/png", | ||
| content_url=f"data:image/png;base64,{image_data}" | ||
| ) |
…into users/robrandao/downloader
Copilot started reviewing on behalf of
Rodrigo Brandão (rodrigobr-msft)
September 10, 2026 15:50
View session
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved security and correctness issues remain in attachment downloading and claims handling.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (18)
libraries/microsoft-agents-activity/microsoft_agents/activity/attachment.py:26
- The model now permits
content_urlandnameto beNone, but the class docstring still documents both fields as plainstr. Generated API documentation will therefore contradict the new nullable contract; update the:typeentries to reflect optional strings.
content_url: NonEmptyString | None = None
content: object = None
name: NonEmptyString | None = None
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/_utils.py:22
- The comment spells
attribtuesincorrectly. Change it toattributes.
# the later ones are the attribtues like "charset"
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/_utils.py:23
get_params()can produce an empty result for an empty or malformed header, but this only checks forNonebefore indexingparams[0]. The downloaders passheaders.get("Content-Type", ""), so a response without a usable Content-Type can raise instead of returningNoneas documented; guard the empty-list case before indexing.
params = email.get_params()
if params is None:
return None
# the first param is the mime-type
# the later ones are the attribtues like "charset"
return params[0][0], dict(params[1:])
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py:66
startswith("http://localhost")also accepts hosts such aslocalhost.evil.example(and user-info forms), so the intended localhost-only HTTP exception is bypassed. Parse the URL and allow HTTP only when the parsed hostname is exactlylocalhost; otherwise require HTTPS before making the request.
if attachment.content_url and (
attachment.content_url.startswith("https://")
or attachment.content_url.startswith("http://localhost")
):
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py:52
- The generic downloader processes every attachment, including
text/html, whereas the M365 downloader explicitly filters those attachments. A normal HTML/card attachment is therefore serialized into anInputFileand exposed as a user-uploaded file; skiptext/htmlattachments here as well.
files: list[InputFile] = []
for attachment in context.activity.attachments:
file = await self._download_file(attachment)
if file:
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py:77
- This creates and closes a new
aiohttp.ClientSessionfor every remote attachment. A turn with multiple files therefore creates one connection pool per file instead of reusing connections, adding setup overhead and increasing resource pressure; create one session perdownload_filescall and reuse it across attachments.
async with self._client_factory() as client:
async with client.get(remote_file_url) as response:
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/attachment_downloader.py:88
- The response bytes are returned unchanged, but every
image/*response is relabeled asimage/png. A JPEG or GIF payload will therefore be advertised as PNG and cannot be rendered or consumed correctly; preserve the response MIME type or actually transcode the bytes before changing it.
if content_type.startswith("image/"):
content_type = "image/png"
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/input_file.py:27
filenameis a new public field, but it is missing from theInputFiledocstring's parameter and type list. Add its optional filename semantics so the documented API matches the dataclass.
filename: str | None = None
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:75
- This exact-string check misses the channel IDs already treated as M365 Copilot elsewhere, such as
msteams:copilot,msteams:copilot-web, andmsteams:copilot:web, and it also misses Teams subchannels. Those turns return no files even though this downloader is configured for M365/Teams. Normalize withChannelId.get_channel(...)(as the generic downloader does) before deciding whether to skip the turn.
if context.activity.channel_id not in (
Channels.ms_teams,
Channels.m365_copilot,
):
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:70
- The downloader is invoked for every configured file downloader, so this identity/audience check runs before the channel no-op and also runs in anonymous mode. As a result, an identity-less non-M365 turn raises instead of returning
[], anduse_anonymous=Truestill cannot process an identity-less context even though no token is needed. Check the channel first and require an identity only for the authenticated path.
if not context.identity:
raise ValueError("No valid context identity found.")
outgoing_audience_claim = context.identity.get_outgoing_audience_claim()
if not outgoing_audience_claim:
raise ValueError("No valid outgoing App ID found.")
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:137
- The scheme/host gate is applied to
attachment.content_url, but a dictionary attachment can replace it with an arbitrarydownloadUrlon the next line. With the default-disabled host validator, a payload with a safe HTTPS content URL can therefore make the client request an unvalidated HTTP or other-scheme endpoint. Validate the finaldownload_url(using parsed scheme/hostname checks) before callingclient.get.
download_url: str
if isinstance(attachment.content, dict):
content_dict = cast(dict[str, Any], attachment.content)
download_url = content_dict.get("downloadUrl", attachment.content_url)
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:151
- In anonymous mode
access_tokenis empty, but this still sendsAuthorization: Bearerinstead of omitting the authorization header. Servers can reject that malformed credential, souse_anonymous=Truemay fail rather than perform an anonymous request. Only include the header whenaccess_tokenis non-empty.
async with client.get(
download_url, headers={"Authorization": f"Bearer {access_token}"}
) as response:
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:133
- The same prefix check accepts non-local hosts in the M365 path, and this path sends the bearer token on the request. An input such as
http://localhost.evil.com/filetherefore reaches the authenticated download branch when validation is disabled; validate the parsed hostname rather than using a string prefix.
if attachment.content_url and (
attachment.content_url.startswith("https://")
or attachment.content_url.startswith("http://localhost")
):
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:149
- This creates and closes a new
aiohttp.ClientSessionfor every remote attachment. A turn with multiple files therefore creates one connection pool per file instead of reusing connections, adding setup overhead and increasing resource pressure; create one session perdownload_filescall and reuse it across attachments.
async with self._client_factory() as client:
async with client.get(
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/app/m365_attachment_downloader.py:162
- The response bytes are returned unchanged, but every
image/*response is relabeled asimage/png. A JPEG or GIF payload will therefore be advertised as PNG and cannot be rendered or consumed correctly; preserve the response MIME type or actually transcode the bytes before changing it.
if content_type.startswith("image/"):
content_type = "image/png"
libraries/microsoft-agents-hosting-core/microsoft_agents/hosting/core/authorization/claims_identity.py:118
- A normal government Bot Framework identity with
verandappidalso satisfiesis_agent_claim()because that helper only excludes the public issuer. It therefore returnsapi://<appid>here and never reaches the government branch, causing M365 token acquisition in Gov to use the wrong audience; checkis_gov_botframework_claim()beforeis_agent_claim()(and cover a v1/v2 Gov claim).
if self.is_agent_claim():
return f"api://{self.get_outgoing_app_id()}"
if self.is_gov_botframework_claim():
return AuthenticationConstants.GOV_AGENTS_SDK_TOKEN_ISSUER
test_samples/handling_attachments/src/agent.py:128
- The sample labels every downloaded file as PNG and embeds a PNG data URI, even though the new downloaders preserve arbitrary content types. Uploading a text, PDF, or other non-image file will therefore be echoed with the wrong MIME type; use the
InputFile.content_typefor both fields.
content_type="image/png",
content_url=f"data:image/png;base64,{image_data}"
tests/hosting_core/app/test_m365_attachment_downloader.py:55
get_connectionreturns the same provider for every name, sotest_uses_named_token_provider_when_configuredwould pass even iftoken_provider_namewere ignored and the activity fallback were used. Make the fake distinguish the named lookup from the fallback (or record the requested name) so this test actually covers the configured-connection branch.
def get_connection(self, connection_name: str) -> AccessTokenProviderBase:
if self._provider is None:
raise ValueError("no connection configured")
return self._provider
- Files reviewed: 19/23 changed files
- Comments generated: 4
- Review effort level: Lite
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request introduces significant enhancements to attachment handling in the Microsoft Agents Hosting Core library, including robust support for downloading attachments from both generic and M365/Teams channels, improved type annotations for attachment-related models, and new utility functions for content type parsing. It also introduces a new method for constructing attachment URIs and updates to claims identity logic for proper audience claim handling.
Attachment Downloading and Handling:
AttachmentDownloaderandM365AttachmentDownloaderclasses to handle downloading attachments from generic and M365/Teams channels, including authentication, content-type handling, and host validation. (microsoft_agents/hosting/core/app/attachment_downloader.py,microsoft_agents/hosting/core/app/m365_attachment_downloader.py) [1] [2]InputFilemodel to support optionalcontent_urlandfilenamefields, and updated the downloader logic to use these fields. (microsoft_agents/hosting/core/app/input_file.py)Content Type and Utility Improvements:
_parse_content_typeutility function to robustly parse MIME types and parameters from content-type headers. (microsoft_agents/hosting/core/app/_utils.py)Attachment Model and Channel Updates:
Attachmentmodel to allowcontent_urlandnameto beNone, improving flexibility in attachment handling. (microsoft_agents/activity/attachment.py)m365_copilotto theChannelsenum for Microsoft 365 Copilot support. (microsoft_agents/activity/channels.py)Claims Identity and Authorization:
get_outgoing_audience_claimandis_gov_botframework_claimmethods toClaimsIdentityfor improved audience claim and government cloud support in token handling. (microsoft_agents/hosting/core/authorization/claims_identity.py) [1] [2]Attachment URI Construction:
get_attachment_urimethod to connector clients for constructing URIs to access attachment views, with error handling and proper URL encoding. (microsoft_agents/hosting/core/connector/attachments_base.py,microsoft_agents/hosting/core/connector/client/connector_client.py,microsoft_agents/hosting/core/connector/mcs/mcs_connector_client.py) [1] [2] [3]Test and Environment Updates:
test_samples/handling_attachments/env.TEMPLATE,test_samples/handling_attachments/requirements.txt) [1] [2]