Skip to content

Commit 2811fba

Browse files
[MCP] Fix for inconsistent MCP enabled check (#3303)
## Why make this change? - Closes #3284 MCP endpoint returns 404 when the mcp config block is omitted, despite mcp.enabled defaulting to true across schema, CLI, and model layers. ## What is this change? `McpEndpointRouteBuilderExtensions.TryGetMcpOptions`: Falls back to new `McpRuntimeOptions()` when `Runtime.Mcp` is `null`, so the `/mcp` endpoint is registered by default. `RestService.GetRouteAfterPathBase`: Guards the `GlobalMcpEndpointDisabled` exception with an `IsMcpEnabled` check so MCP path requests are not unconditionally rejected. ## How was this tested? - [ ] Integration Tests - [x] Unit Tests ## Sample Request(s) /mcp /health
1 parent 16fdda3 commit 2811fba

5 files changed

Lines changed: 153 additions & 25 deletions

File tree

src/Azure.DataApiBuilder.Mcp/Core/McpEndpointRouteBuilderExtensions.cs

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,14 @@ public static IEndpointRouteBuilder MapDabMcp(
2222
RuntimeConfigProvider runtimeConfigProvider,
2323
[StringSyntax("Route")] string pattern = "")
2424
{
25-
if (!TryGetMcpOptions(runtimeConfigProvider, out McpRuntimeOptions? mcpOptions) || mcpOptions == null || !mcpOptions.Enabled)
25+
if (!runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig))
26+
{
27+
return endpoints;
28+
}
29+
30+
McpRuntimeOptions mcpOptions = runtimeConfig?.Runtime?.Mcp ?? new McpRuntimeOptions();
31+
32+
if (!mcpOptions.Enabled)
2633
{
2734
return endpoints;
2835
}
@@ -34,24 +41,5 @@ public static IEndpointRouteBuilder MapDabMcp(
3441

3542
return endpoints;
3643
}
37-
38-
/// <summary>
39-
/// Gets MCP options from the runtime configuration
40-
/// </summary>
41-
/// <param name="runtimeConfigProvider">Runtime config provider</param>
42-
/// <param name="mcpOptions">MCP options</param>
43-
/// <returns>True if MCP options were found, false otherwise</returns>
44-
private static bool TryGetMcpOptions(RuntimeConfigProvider runtimeConfigProvider, out McpRuntimeOptions? mcpOptions)
45-
{
46-
mcpOptions = null;
47-
48-
if (!runtimeConfigProvider.TryGetConfig(out RuntimeConfig? runtimeConfig))
49-
{
50-
return false;
51-
}
52-
53-
mcpOptions = runtimeConfig?.Runtime?.Mcp;
54-
return mcpOptions != null;
55-
}
5644
}
5745
}

src/Cli/ConfigGenerator.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2653,7 +2653,7 @@ public static bool IsConfigValid(ValidateOptions options, FileSystemRuntimeConfi
26532653
{
26542654
if (runtimeConfigProvider.TryGetConfig(out RuntimeConfig? config) && config is not null)
26552655
{
2656-
bool mcpEnabled = config.Runtime?.Mcp?.Enabled == true;
2656+
bool mcpEnabled = config.IsMcpEnabled;
26572657
if (mcpEnabled)
26582658
{
26592659
foreach (KeyValuePair<string, Entity> entity in config.Entities)

src/Core/Services/RestService.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -402,15 +402,17 @@ private bool TryGetStoredProcedureRESTVerbs(string entityName, [NotNullWhen(true
402402
/// does not match the configured REST path or the global REST endpoint is disabled.</exception>
403403
public string GetRouteAfterPathBase(string route)
404404
{
405-
string configuredRestPathBase = _runtimeConfigProvider.GetConfig().RestPath;
405+
RuntimeConfig runtimeConfig = _runtimeConfigProvider.GetConfig();
406+
string configuredRestPathBase = runtimeConfig.RestPath;
406407

407408
// Strip the leading '/' from the REST path provided in the runtime configuration
408409
// because the input argument 'route' has no starting '/'.
409410
// The RuntimeConfigProvider enforces the expectation that the configured REST path starts with a
410411
// forward slash '/'.
411412
configuredRestPathBase = configuredRestPathBase.Substring(1);
412413

413-
if (route.Equals(_runtimeConfigProvider.GetConfig().McpPath.Substring(1)))
414+
if (route.Equals(runtimeConfig.McpPath.Substring(1))
415+
&& !runtimeConfig.IsMcpEnabled)
414416
{
415417
throw new DataApiBuilderException(
416418
message: $"Route {route} was not found.",

src/Service.Tests/Configuration/McpRuntimeOptionsSerializationTests.cs

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,94 @@ public void TestBackwardCompatibilityDeserializationWithoutDescriptionField()
191191
Assert.IsNull(deserializedConfig.Runtime.Mcp.Description, "Description should be null when not present in JSON");
192192
}
193193

194+
/// <summary>
195+
/// When the mcp and runtime config blocks are omitted entirely, IsMcpEnabled should default to true.
196+
/// </summary>
197+
[TestMethod]
198+
public void TestIsMcpEnabledDefaultsTrueWhenMcpAndRuntimeBlocksAbsent()
199+
{
200+
// Arrange - config with no runtime/mcp block
201+
string configJson = @"{
202+
""$schema"": ""test-schema"",
203+
""data-source"": {
204+
""database-type"": ""mssql"",
205+
""connection-string"": ""Server=test;Database=test;""
206+
},
207+
""entities"": {}
208+
}";
209+
210+
// Act
211+
bool parseSuccess = RuntimeConfigLoader.TryParseConfig(configJson, out RuntimeConfig config);
212+
213+
// Assert
214+
Assert.IsTrue(parseSuccess);
215+
Assert.IsNull(config.Runtime, "Runtime should be null when not specified");
216+
Assert.IsNull(config.Runtime?.Mcp, "Mcp should be null when not specified");
217+
Assert.IsTrue(config.IsMcpEnabled, "IsMcpEnabled should default to true when mcp block is absent");
218+
Assert.AreEqual(McpRuntimeOptions.DEFAULT_PATH, config.McpPath, "McpPath should return default when mcp block is absent");
219+
}
220+
221+
/// <summary>
222+
/// When the mcp config block is present but enabled is not specified,
223+
/// IsMcpEnabled should default to true.
224+
/// </summary>
225+
[TestMethod]
226+
public void TestIsMcpEnabledDefaultsTrueWhenEnabledNotSpecified()
227+
{
228+
// Arrange - config with mcp block but no enabled field
229+
string configJson = @"{
230+
""$schema"": ""test-schema"",
231+
""data-source"": {
232+
""database-type"": ""mssql"",
233+
""connection-string"": ""Server=test;Database=test;""
234+
},
235+
""runtime"": {
236+
""mcp"": {
237+
""path"": ""/mcp""
238+
}
239+
},
240+
""entities"": {}
241+
}";
242+
243+
// Act
244+
bool parseSuccess = RuntimeConfigLoader.TryParseConfig(configJson, out RuntimeConfig config);
245+
246+
// Assert
247+
Assert.IsTrue(parseSuccess);
248+
Assert.IsNotNull(config.Runtime?.Mcp);
249+
Assert.IsTrue(config.IsMcpEnabled, "IsMcpEnabled should default to true when enabled is not specified");
250+
}
251+
252+
/// <summary>
253+
/// When mcp.enabled is explicitly set to false, IsMcpEnabled should return false.
254+
/// </summary>
255+
[TestMethod]
256+
public void TestIsMcpEnabledReturnsFalseWhenExplicitlyDisabled()
257+
{
258+
// Arrange
259+
string configJson = @"{
260+
""$schema"": ""test-schema"",
261+
""data-source"": {
262+
""database-type"": ""mssql"",
263+
""connection-string"": ""Server=test;Database=test;""
264+
},
265+
""runtime"": {
266+
""mcp"": {
267+
""enabled"": false
268+
}
269+
},
270+
""entities"": {}
271+
}";
272+
273+
// Act
274+
bool parseSuccess = RuntimeConfigLoader.TryParseConfig(configJson, out RuntimeConfig config);
275+
276+
// Assert
277+
Assert.IsTrue(parseSuccess);
278+
Assert.IsNotNull(config.Runtime?.Mcp);
279+
Assert.IsFalse(config.IsMcpEnabled, "IsMcpEnabled should be false when explicitly disabled");
280+
}
281+
194282
/// <summary>
195283
/// Creates a minimal RuntimeConfig with the specified MCP options for testing.
196284
/// </summary>

src/Service.Tests/UnitTests/RestServiceUnitTests.cs

Lines changed: 52 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,46 @@ public void SinglePathMatchingTest()
167167

168168
#endregion
169169

170+
#region MCP Path Guard Tests
171+
172+
/// <summary>
173+
/// When MCP is explicitly disabled and the route matches the MCP path (default or custom),
174+
/// GetRouteAfterPathBase should throw GlobalMcpEndpointDisabled.
175+
/// </summary>
176+
[DataTestMethod]
177+
[DataRow("/mcp", "mcp", DisplayName = "MCP disabled with default path")]
178+
[DataRow("/custom-mcp", "custom-mcp", DisplayName = "MCP disabled with custom path")]
179+
public void McpPathThrowsWhenMcpDisabled(string mcpPath, string route)
180+
{
181+
InitializeTestWithMcpConfig("/api", new McpRuntimeOptions(Enabled: false, Path: mcpPath));
182+
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(
183+
() => _restService.GetRouteAfterPathBase(route));
184+
Assert.AreEqual(HttpStatusCode.NotFound, ex.StatusCode);
185+
Assert.AreEqual(DataApiBuilderException.SubStatusCodes.GlobalMcpEndpointDisabled, ex.SubStatusCode);
186+
}
187+
188+
/// <summary>
189+
/// When MCP is enabled (explicitly or by default when config is absent),
190+
/// the MCP path route should NOT throw GlobalMcpEndpointDisabled.
191+
/// It falls through to the normal path-base check.
192+
/// </summary>
193+
[DataTestMethod]
194+
[DataRow(true, DisplayName = "MCP explicitly enabled")]
195+
[DataRow(null, DisplayName = "MCP config absent (defaults to enabled)")]
196+
public void McpPathDoesNotThrowGlobalMcpEndpointDisabledWhenMcpEnabled(bool? mcpEnabled)
197+
{
198+
McpRuntimeOptions mcpOptions = mcpEnabled.HasValue ? new McpRuntimeOptions(Enabled: mcpEnabled.Value) : null;
199+
InitializeTestWithMcpConfig("/api", mcpOptions);
200+
// "mcp" doesn't start with "api", so it should throw BadRequest (invalid path),
201+
// NOT GlobalMcpEndpointDisabled.
202+
DataApiBuilderException ex = Assert.ThrowsException<DataApiBuilderException>(
203+
() => _restService.GetRouteAfterPathBase("mcp"));
204+
Assert.AreEqual(HttpStatusCode.BadRequest, ex.StatusCode);
205+
Assert.AreEqual(DataApiBuilderException.SubStatusCodes.BadRequest, ex.SubStatusCode);
206+
}
207+
208+
#endregion
209+
170210
#region Helper Functions
171211

172212
/// <summary>
@@ -215,15 +255,15 @@ public static void InitializeTestWithMultipleEntityPaths(string restRoutePrefix,
215255
/// <summary>
216256
/// Core helper to initialize REST Service with specified entity path mappings.
217257
/// </summary>
218-
private static void InitializeTestWithEntityPaths(string restRoutePrefix, Dictionary<string, string> entityPaths)
258+
private static void InitializeTestWithEntityPaths(string restRoutePrefix, Dictionary<string, string> entityPaths, McpRuntimeOptions mcpOptions = null, bool useMcpParam = false)
219259
{
220260
RuntimeConfig mockConfig = new(
221261
Schema: "",
222262
DataSource: new(DatabaseType.PostgreSQL, "", new()),
223263
Runtime: new(
224264
Rest: new(Path: restRoutePrefix),
225265
GraphQL: new(),
226-
Mcp: new(),
266+
Mcp: useMcpParam ? mcpOptions : (mcpOptions ?? new()),
227267
Host: new(null, null)
228268
),
229269
Entities: new(new Dictionary<string, Entity>())
@@ -306,6 +346,16 @@ private static void InitializeTestWithEntityPaths(string restRoutePrefix, Dictio
306346
provider,
307347
requestValidator);
308348
}
349+
350+
/// <summary>
351+
/// Initializes RestService with a specific MCP configuration for testing MCP path guard behavior.
352+
/// </summary>
353+
/// <param name="restRoutePrefix">REST path prefix (e.g., "/api").</param>
354+
/// <param name="mcpOptions">MCP options, or null to simulate absent mcp config block.</param>
355+
private static void InitializeTestWithMcpConfig(string restRoutePrefix, McpRuntimeOptions mcpOptions)
356+
{
357+
InitializeTestWithEntityPaths(restRoutePrefix, new Dictionary<string, string>(), mcpOptions, useMcpParam: true);
358+
}
309359
#endregion
310360
}
311361
}

0 commit comments

Comments
 (0)