[Lily Du (lilyydu)]

related issue: microsoft/teams.ts#985

Summary

• Route Socket Mode negotiation through https://{geo}.botapi.skype.com/v3/websockets/connect.
• Remove configurable negotiate base URLs.
• Default empty geo configurations to amer, emea, and apac.
• Accept custom non-empty geo values while rejecting empty strings and duplicates.
• Add secret-safe request URL diagnostics.
• Update Socket Mode tests and documentation.

Validation

• Ruff and Pyright pass.
• 118 Socket Mode tests pass.
• Live validation reached ready for amer, emea, and apac.

[Copilot]

🟡 Changes recommended

Unvalidated geo values can redirect negotiation to an attacker-controlled host, exposing the bot's bearer token.

3 open issues

• Validate geo labels before resolving negotiation URLs · New
• Redact credentials from negotiation URL debug logs · New
• Prefix only the hostname when applying geo routing · New

What changed in this PR

Updates Python Socket Mode negotiation to use regional hosts, addressing the missed cross-region activities described in teams.ts#985.

Changes:

• Replace geo path segments with geo-prefixed service hosts.
• Add DEBUG logging for resolved negotiation URLs.
• Update Socket Mode URL assertions, HTTP mocks, and log capture.

File	Description
packages/​apps/​tests/​test_socket_mode_transport.py	Checks regional URLs and diagnostic logs; updates geo extraction and log capture.
packages/​apps/​tests/​test_socket_mode_connection.py	Allows geo-prefixed custom hosts in HTTP mocks.
packages/​apps/​tests/​test_socket_mode_adapter.py	Updates URL expectations for default and custom service hosts.
packages/​apps/​src/​microsoft_teams/​apps/​socket_mode/​transport.py	Builds geo-specific negotiation URLs and logs each resolved endpoint.

_{Review effort: Max}

[Arup Roy (aroy-ms)]

Approving with some suggestions, let's make sure we're not logging username:pwd if that's in the negotiate url (per the copilot comments)

[Copilot]

🟡 Changes recommended

Tests no longer verify first-geo selection and lack direct coverage of invalid-geo rejection and base-path/query preservation in URL construction.

3 open issues

• Use multiple geos to verify first-entry URL selection · New
• Test invalid geos and preservation of base URL components · New
• Document regional URL format, host restrictions, and geo allowlist · New

3 resolved since last review

• Validate geo labels before resolving negotiation URLs
• Prefix only the hostname when applying geo routing
• Redact credentials from negotiation URL debug logs

_{Review effort: Max}

[Copilot]

🔵 Needs a closer look

Negotiation URL handling can alter query values and leave the transport reporting CONNECTING with accepting enabled after startup fails on a malformed URL.

No open issues 🎉

2 previously missed issues

In code that hasn't changed since last review

Malformed negotiate URL leaves transport stuck connecting

packages/​apps/​src/​microsoft_teams/​apps/​socket_mode/​transport.py:204

If negotiate_base_url is malformed (for example, https://[botapi.example), urlsplit() raises after the transport has entered CONNECTING. Startup then fails with no sockets, but status still reports CONNECTING and accepting remains true. Resolve all negotiate URLs before changing the stop event or lifecycle.

Trimming the base URL corrupts query parameters

packages/​apps/​src/​microsoft_teams/​apps/​socket_mode/​transport.py:355

For a custom base URL ending in a query value such as https://botapi.example/ring?route=/, trimming the entire URL before parsing changes the query from route=/ to route=. The negotiate request therefore receives a different parameter value despite the intended query preservation. Parse base_url directly and trim only parsed.path, which is already handled below.

3 resolved since last review

• Test invalid geos and preservation of base URL components
• Use multiple geos to verify first-entry URL selection
• Document regional URL format, host restrictions, and geo allowlist

_{Review effort: Max}

[Copilot]

🔵 Needs a closer look

Mocked tests cannot verify live Teams DNS, routing, and token acceptance for the new Socket Mode hosts, and the change also narrows supported geos and negotiate_base_url values.

No open issues 🎉

_{Review effort: Max}

[Copilot]

🟡 Changes recommended

The implementation and tests use a different regional hostname than the PR summary, leaving unclear whether live verification covered the endpoint being shipped.

1 open issue

• Reconcile the shipped regional hostname with the PR description · New

_{Review effort: Max}

[Copilot]

🔵 Needs a closer look

Human assessment is needed to evaluate the compatibility impact of removing negotiate_base_url, which breaks existing callers and removes the documented backend-ring selection mechanism.

No open issues 🎉

1 resolved since last review

• Reconcile the shipped regional hostname with the PR description

_{Review effort: Max}

[Copilot]

🟡 Changes recommended

Custom geo values remain unvalidated, allowing negotiation requests to expose the bot’s bearer token to an attacker-controlled host.

1 open issue

• Restrict custom geos to the allowlist to prevent bearer-token exfiltration · New

_{Review effort: Max}

[Copilot]

🔵 Needs a closer look

Human assessment is needed because geo-validation edge cases can cause negotiation failures or duplicate connections to the same region.

No open issues 🎉

1 resolved since last review

• Restrict custom geos to the allowlist to prevent bearer-token exfiltration

_{Review effort: Max}

[Arup Roy (aroy-ms)]

approving latest iter

[Copilot]

🔵 Needs a closer look

A moderate-severity geo-validation regression was omitted from the published review and needs human assessment of its compatibility impact.

No open issues 🎉

_{Review effort: Max}