Skip to content

Add context-aware CORS policy to enhance security #1609

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.

Sign up for GitHub

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

Open
g2vinay wants to merge 3 commits into
base: main
Choose a base branch
from
+67 −28
Open

Add context-aware CORS policy to enhance security #1609

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
eacdb8e
Add context-aware CORS policy for security
g2vinay Jan 29, 2026
ee83aff
format fix
g2vinay Feb 3, 2026
eb4371c
add changelog
g2vinay Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 64 additions & 28 deletions core/Azure.Mcp.Core/src/Areas/Server/Commands/ServiceStartCommand.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,18 +523,10 @@ private IHost CreateHttpHost(ServiceStartOptions serverOptions)
services.AddHealthChecks();

// Configure CORS
// We're allowing all origins, methods, and headers to support any web
// browser clients.
// By default in development mode, we restrict to localhost origins for security.
// In production (authenticated mode), allow configured origins or all origins if specified.
// Non-browser clients are unaffected by CORS.
services.AddCors(options =>
{
options.AddPolicy("AllowAll", policy =>
{
policy.AllowAnyOrigin()
.AllowAnyMethod()
.AllowAnyHeader();
});
});
ConfigureCors(services, serverOptions);

// Configure services
ConfigureServices(services); // Our static callback hook
Expand All @@ -545,7 +537,7 @@ private IHost CreateHttpHost(ServiceStartOptions serverOptions)
UseHttpsRedirectionIfEnabled(app);

// Configure middleware pipeline
app.UseCors("AllowAll");
app.UseCors("McpCorsPolicy");
app.UseRouting();

// Add OAuth protected resource metadata middleware
Expand Down Expand Up @@ -640,18 +632,10 @@ private IHost CreateIncomingAuthDisabledHttpHost(ServiceStartOptions serverOptio
services.AddSingleIdentityTokenCredentialProvider();

// Configure CORS
// We're allowing all origins, methods, and headers to support any web
// browser clients.
// By default in development mode, we restrict to localhost origins for security.
// In production (authenticated mode), allow configured origins or all origins if specified.
// Non-browser clients are unaffected by CORS.
services.AddCors(options =>
{
options.AddPolicy("AllowAll", policy =>
{
policy.AllowAnyOrigin()
.AllowAnyMethod()
.AllowAnyHeader();
});
});
ConfigureCors(services, serverOptions);

// Configure services
ConfigureServices(services); // Our static callback hook
Expand All @@ -668,13 +652,65 @@ private IHost CreateIncomingAuthDisabledHttpHost(ServiceStartOptions serverOptio
UseHttpsRedirectionIfEnabled(app);

// Configure middleware pipeline
app.UseCors("AllowAll");
app.UseCors("McpCorsPolicy");
app.UseRouting();
app.MapMcp();

return app;
}

/// <summary>
/// Configures CORS policy based on environment and configuration.
/// In development mode (unauthenticated), restricts to localhost for security.
/// In production (authenticated), allows all origins (safe due to authentication requirement).
/// </summary>
/// <param name="services">The service collection to configure.</param>
/// <param name="serverOptions">The server configuration options.</param>
private static void ConfigureCors(IServiceCollection services, ServiceStartOptions serverOptions)
{
services.AddCors(options =>
{
options.AddPolicy("McpCorsPolicy", policy =>
{
// In unauthenticated mode (development), restrict to localhost by default
// Allows localhost with any port to support various development scenarios:
// - Port 1031: Default HTTP mode (launchSettings.json)
// - Port 5008: SSE mode default
// - Port 5173: MCP Inspector tool
// - Custom ports: User-specified via ASPNETCORE_URLS
if (serverOptions.DangerouslyDisableHttpIncomingAuth)
{
policy.SetIsOriginAllowed(origin =>
{
if (Uri.TryCreate(origin, UriKind.Absolute, out var uri))
{
// Allow localhost and 127.0.0.1 with any port
return uri.Host == "localhost" ||
uri.Host == "127.0.0.1" ||
uri.Host == "[::1]"; // IPv6 loopback
}
return false;
})
.AllowAnyMethod()
.AllowAnyHeader()
.AllowCredentials(); // Required when using SetIsOriginAllowed
Comment on lines +684 to +696
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The lambda expression passed to SetIsOriginAllowed has inconsistent indentation. The opening brace on line 684 is indented with extra spaces compared to the standard indentation pattern used in the rest of the codebase. This should be aligned consistently with the method call on line 683.

Suggested change
{
if (Uri.TryCreate(origin, UriKind.Absolute, out var uri))
{
// Allow localhost and 127.0.0.1 with any port
return uri.Host == "localhost" ||
uri.Host == "127.0.0.1" ||
uri.Host == "[::1]"; // IPv6 loopback
}
return false;
})
.AllowAnyMethod()
.AllowAnyHeader()
.AllowCredentials(); // Required when using SetIsOriginAllowed
{
if (Uri.TryCreate(origin, UriKind.Absolute, out var uri))
{
// Allow localhost and 127.0.0.1 with any port
return uri.Host == "localhost" ||
uri.Host == "127.0.0.1" ||
uri.Host == "[::1]"; // IPv6 loopback
}
return false;
})
.AllowAnyMethod()
.AllowAnyHeader()
.AllowCredentials(); // Required when using SetIsOriginAllowed

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The comment states that AllowCredentials is "Required when using SetIsOriginAllowed", but this is not accurate. While AllowCredentials is commonly used with SetIsOriginAllowed for authenticated scenarios, it is not technically required by the API. The comment should clarify that it's needed to allow browsers to send credentials (cookies, authorization headers) in cross-origin requests, which is important for the development mode scenario.

Suggested change
.AllowCredentials(); // Required when using SetIsOriginAllowed
.AllowCredentials(); // Allows browsers to send credentials (cookies, auth headers) with cross-origin requests when using this localhost-only policy in development

Copilot uses AI. Check for mistakes.
}
// In authenticated mode (production), allow all origins by default
// This is safe because:
// 1. Authentication (JWT Bearer) validates all requests regardless of origin
// 2. CORS is a browser security mechanism, not a server security feature
// 3. MCP clients (GitHub Copilot in VS Code/Codespaces) need to connect from various origins
// 4. The server still enforces authentication and authorization on every request
else
{
policy.AllowAnyOrigin()
.AllowAnyMethod()
.AllowAnyHeader();
}
});
});
}
Comment on lines +669 to +712
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

The new ConfigureCors method introduces important security logic that restricts localhost origins in development mode but lacks unit test coverage. Given that the ServiceStartCommandTests.cs file exists with tests for other configuration options, tests should be added to verify:

  1. Localhost origins (localhost, 127.0.0.1, [::1]) are allowed when DangerouslyDisableHttpIncomingAuth is true
  2. Non-localhost origins are rejected when DangerouslyDisableHttpIncomingAuth is true
  3. All origins are allowed when DangerouslyDisableHttpIncomingAuth is false
  4. AllowCredentials is set correctly in development mode

Copilot uses AI. Check for mistakes.

/// <summary>
/// Configures the MCP server services.
/// </summary>
Expand Down Expand Up @@ -852,11 +888,11 @@ private static WebApplication UseHttpsRedirectionIfEnabled(WebApplication app)
}

return Sdk.CreateTracerProviderBuilder()
// captures incoming HTTP requests
.AddAspNetCoreInstrumentation()
// captures outgoing HTTP requests with filtering
.AddHttpClientInstrumentation(o => o.FilterHttpRequestMessage = ShouldInstrumentHttpRequest)
.AddAzureMonitorTraceExporter(exporterOptions => exporterOptions.ConnectionString = connectionString)
.AddHttpClientInstrumentation(o =>
o.FilterHttpRequestMessage = ShouldInstrumentHttpRequest)
.AddAzureMonitorTraceExporter(exporterOptions =>
exporterOptions.ConnectionString = connectionString)
Comment on lines +892 to +895
Copy link

Copilot AI Feb 3, 2026

Choose a reason for hiding this comment

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

These formatting changes to the AddHttpClientInstrumentation and AddAzureMonitorTraceExporter method calls appear to be unrelated to the CORS policy changes described in the PR. These changes should either be removed or explained in the PR description if they are intentional refactoring.

Suggested change
.AddHttpClientInstrumentation(o =>
o.FilterHttpRequestMessage = ShouldInstrumentHttpRequest)
.AddAzureMonitorTraceExporter(exporterOptions =>
exporterOptions.ConnectionString = connectionString)
// Configure HTTP client instrumentation to filter out specific requests (e.g., App Insights ingestion, Azure SDK duplicates).
.AddHttpClientInstrumentation(o =>
{
o.FilterHttpRequestMessage = ShouldInstrumentHttpRequest;
})
// Configure Azure Monitor exporter with the connection string from environment configuration.
.AddAzureMonitorTraceExporter(exporterOptions =>
{
exporterOptions.ConnectionString = connectionString;
})

Copilot uses AI. Check for mistakes.
.Build();
}

Expand Down
3 changes: 3 additions & 0 deletions servers/Azure.Mcp.Server/changelog-entries/1770078551923.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
changes:
- section: "Bugs Fixed"
description: "Added CORS policy to restrict cross-origin requests to localhost when running in unauthenticated development environment"