Skip to content

Remove Socket Mode URL diagnostic and reject case-variant geos - #730

Merged
teddy arida-moody (teddyam) merged 1 commit into
mainfrom
teddyam-socket-mode-url-exposure
Nov 10, 2026
Merged

teddy arida-moody (teddyam) merged 1 commit into
mainfrom
teddyam-socket-mode-url-exposure

Conversation

@teddyam

Copy link
Copy Markdown
Contributor

Socket Mode should not expose its temporary negotiation endpoint through a public diagnostic API. Geo names that differ only by case also resolve to the same hostname and must not create independently supervised connections to that host.

Changes

  • Remove SocketModeObserver.NegotiateUrl and its unused internal getter, along with related documentation and assertions. Connection behavior and startup URL logging are unchanged. External callers using this experimental property must remove that usage.
  • Reject duplicate geos using ordinal, case-insensitive comparison while preserving support for individual uppercase geo names and their diagnostic casing.
  • Move ["amer", "AMER"] into rejection coverage and retain a successful single-geo "AMER" case. Document the uniqueness requirement.

Validation

  • Confirmed the mixed-case duplicate regression fails before the fix on both target frameworks.
  • All 225 Socket Mode tests pass on .NET 8 and .NET 10.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI balanced review requested due to automatic review settings November 10, 2026 02:00
@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines:
There may be pipelines that require an authorized user to comment /azp run to run.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues are blocking approval.

No open issues 🎉

What changed in this PR

This PR narrows Socket Mode’s experimental diagnostic API and prevents case-variant geo names from creating separate connections to the same hostname.

Changes:

  • Remove NegotiateUrl from the public observer and internal transport, along with related documentation and assertions.
  • Reject duplicate geos using ordinal, case-insensitive comparison while preserving individual uppercase geo names.
  • Update regression coverage and document the geo uniqueness requirement.
File Description
test/​Microsoft.Teams.Apps.UnitTests/​SocketMode/​SocketModeTransportTests.cs Tests mixed-case duplicate rejection and single uppercase geo support.
test/​Microsoft.Teams.Apps.UnitTests/​SocketMode/​SocketModeObserverTests.cs Removes the obsolete URL diagnostic assertion.
test/​Microsoft.Teams.Apps.UnitTests/​SocketMode/​SocketModeHostingTests.cs Removes the obsolete URL assertion from hosting coverage.
src/​Microsoft.Teams.Apps/​SocketMode/​SocketModeTransport.cs Enforces case-insensitive uniqueness and removes the internal URL getter.
src/​Microsoft.Teams.Apps/​SocketMode/​SocketModeOptions.cs Documents non-empty, case-insensitively unique geo entries.
src/​Microsoft.Teams.Apps/​SocketMode/​SocketModeObserver.cs Removes the public NegotiateUrl property.
src/​Microsoft.Teams.Apps/​README.md Updates the list of available diagnostics.

Review effort: Balanced

@teddyam
teddy arida-moody (teddyam) added this pull request to the merge queue Nov 10, 2026
Merged via the queue into main with commit c9a905a Nov 10, 2026
7 checks passed
@teddyam
teddy arida-moody (teddyam) deleted the teddyam-socket-mode-url-exposure branch November 10, 2026 02:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants