-
Notifications
You must be signed in to change notification settings - Fork 279
Unsecure transport #310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Unsecure transport #310
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces HTTP transport support for the MCP server as an alternative to the default STDIO transport. The changes add a new command-line option --enable-insecure-transports that allows the server to run over HTTP with CORS enabled for development purposes.
Key changes:
- Adds HTTP transport capability with CORS configuration
- Introduces security validation requiring production credentials when enabling insecure transports
- Updates package dependencies to support the new HTTP transport functionality
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| Azure.Mcp.Core.csproj | Adds ModelContextProtocol.AspNetCore package reference for HTTP transport |
| ServiceStartOptions.cs | Adds EnableInsecureTransports property to configuration options |
| ServiceOptionDefinitions.cs | Defines the new command-line option for enabling insecure transports |
| ServiceStartCommand.cs | Implements HTTP host creation and security validation logic |
| ServiceCollectionExtensions.cs | Configures MCP server with appropriate transport based on options |
| Directory.Packages.props | Updates ModelContextProtocol package versions and adds AspNetCore variant |
Comments suppressed due to low confidence (1)
core/Azure.Mcp.Core/src/Areas/Server/Commands/ServiceStartCommand.cs:1
- According to the coding guidelines, System.Text.Json should be used instead of Newtonsoft.Json. Consider migrating to System.Text.Json for consistency.
// Copyright (c) Microsoft Corporation.
core/Azure.Mcp.Core/src/Areas/Server/Commands/ServiceStartCommand.cs
Outdated
Show resolved
Hide resolved
core/Azure.Mcp.Core/src/Areas/Server/Commands/ServiceStartCommand.cs
Outdated
Show resolved
Hide resolved
|
@LarryOsterman, here is the pr based on our offline discussion |
core/Azure.Mcp.Core/src/Areas/Server/Commands/ServiceStartCommand.cs
Outdated
Show resolved
Hide resolved
|
Will this trigger the security concern again? |
bff1a07 to
9ba35aa
Compare
core/Azure.Mcp.Core/src/Areas/Server/Commands/ServiceStartCommand.cs
Outdated
Show resolved
Hide resolved
9ba35aa to
8cfa11c
Compare
8cfa11c to
43b2bcc
Compare
* Updated live tests * Added unit test for list keys * Added unit tests for get key * Added unit tests for create key * Refactored code and removed unnecessary tests
* Enable enable-insecure-transports option * Check for ProdCred for streaming * allow standard ASPNETCORE_URLS and fix message * address cspell * improve message * adding GetSafeAspNetCoreUrl * restrict further
What does this PR do?
[Provide a clear, concise description of the changes]adding an insecure, undocumented switch to begin the early exploration of self-hosted remote mcp server.
[Any additional context, screenshots, or information that helps reviewers]GitHub issue number?
[Link to the GitHub issue this PR addresses]Pre-merge Checklist
CHANGELOG.mdfor product changes (features, bug fixes, UI/UX, updated dependencies).\eng\common\spelling\Invoke-Cspell.ps1README.mddocumentation/docs/azmcp-commands.md/docs/e2eTestPrompts.mdToolDescriptionEvaluatorand obtained a score of0.4or more and a top 3 ranking for all related test promptscrypto mining, spam, data exfiltration, etc.)/azp run mcp - pullrequest - liveto run Live Test Pipeline