Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
7 changes: 6 additions & 1 deletion src/CrestApps.Core.Docs/docs/changelog/1.1.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ page will be updated as changes land after 1.0.0.
- reworks tool exposure into an opt-in allow-list driven by the `McpServerOptions` site settings. Nothing
is exposed by default: an MCP server lists and invokes only the tools and configured
[tool instances](../core/tool-instances.md) named in `McpServerOptions.Tools`, or every tool
and instance when `McpServerOptions.ExposeAllTools` is `true`. The allow-list is enforced by both the
and instance when `McpServerOptions.ExposeAllTools` is `true`. Only selectable tools participate: system
tools (marked `IsSystemTool`) and hidden tools are never exposed, even when `ExposeAllTools` is `true`.
The allow-list is enforced by both the
list and call handlers, so a tool that is not exposed can neither be discovered nor invoked. Because
`McpServerOptions` is backed by site settings, operators choose which tools to expose from the admin
settings UI without redeploying. The MVC and Blazor sample hosts add an "Exposed tools" editor to their
Expand Down Expand Up @@ -65,6 +67,9 @@ page will be updated as changes land after 1.0.0.

## Fixes

- stops the MCP server from exposing system tools. `WithCrestAppsHandlers()` now lists and invokes only
selectable tools, so tools marked `IsSystemTool` (which agents auto-include based on context) are never
discoverable or callable over MCP, even when `McpServerOptions.ExposeAllTools` is `true`
- makes the MCP tool allow-list respond to site-settings changes at runtime. The list and call handlers
now read `McpServerOptions` through `IOptionsMonitor` instead of the cached `IOptions`, so exposing or
removing a tool from the admin settings page takes effect without restarting the host
Expand Down
4 changes: 3 additions & 1 deletion src/CrestApps.Core.Docs/docs/mcp/server.md
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,9 @@ services.Configure<McpServerOptions>(options =>
| Property | Effect |
|----------|--------|
| `Tools` | An allow-list of tool and tool instance names to expose. Matching is case-insensitive. |
| `ExposeAllTools` | When `true`, every tool and tool instance is exposed and the allow-list is ignored. |
| `ExposeAllTools` | When `true`, every **selectable** tool and tool instance is exposed and the allow-list is ignored. |

Only selectable tools are ever exposed over MCP. System tools (those marked `IsSystemTool`, which agents auto-include based on context) and hidden tools are never listed or callable — not even when `ExposeAllTools` is `true`.

Because `McpServerOptions` is backed by site settings, an operator can choose which tools to expose from the admin **Settings → MCP server** page without redeploying. The allow-list is enforced by **both** the list and call handlers, so a tool that is not exposed can neither be discovered nor invoked.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,10 @@ public static class McpServerBuilderExtensions
/// This wires the CrestApps tool registry (<see cref="AIToolDefinitionOptions"/>),
/// <see cref="IMcpServerPromptService"/>, and <see cref="IMcpServerResourceService"/>
/// into the MCP protocol so both Orchard Core and standalone MVC hosts share the same handler logic.
/// Which tools and tool instances are actually listed and callable is controlled by the
/// <see cref="McpServerOptions"/> site settings allow-list.
/// Only selectable tools (those that are neither system tools nor hidden) are ever exposed, so
/// system tools that the orchestrator auto-includes are never listed or callable over MCP.
/// Which of those selectable tools and tool instances are actually listed and callable is further
/// controlled by the <see cref="McpServerOptions"/> site settings allow-list.
/// </summary>
/// <param name="builder">The builder.</param>
public static IMcpServerBuilder WithCrestAppsHandlers(this IMcpServerBuilder builder)
Expand All @@ -43,7 +45,7 @@ public static IMcpServerBuilder WithCrestAppsHandlers(this IMcpServerBuilder bui

foreach (var (name, definition) in toolDefinitions.Tools)
{
if (definition.Hidden || !IsAllowed(exposeAll, allowList, name, definition.Name))
if (!definition.IsSelectable() || !IsAllowed(exposeAll, allowList, name, definition.Name))
{
continue;
}
Expand Down Expand Up @@ -248,7 +250,7 @@ private static AIFunction ResolveAllowedCodeTool(
ILogger logger)
{
if (toolDefinitions.Tools.TryGetValue(protocolName, out var direct) &&
!direct.Hidden &&
direct.IsSelectable() &&
IsAllowed(exposeAll, allowList, protocolName, direct.Name) &&
TryCreateFunction(services, protocolName, logger) is { } directFunction &&
string.Equals(directFunction.Name, protocolName, StringComparison.Ordinal))
Expand All @@ -258,7 +260,7 @@ private static AIFunction ResolveAllowedCodeTool(

foreach (var (name, definition) in toolDefinitions.Tools)
{
if (definition.Hidden || !IsAllowed(exposeAll, allowList, name, definition.Name))
if (!definition.IsSelectable() || !IsAllowed(exposeAll, allowList, name, definition.Name))
{
continue;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ public async Task ListToolsHandler_DefaultDeny_ReturnsEmpty()
}

/// <summary>
/// Verifies that enabling <c>ExposeAllTools</c> lists every non-hidden tool while hidden tools are omitted.
/// Verifies that enabling <c>ExposeAllTools</c> lists every selectable tool while hidden and system
/// tools are omitted, because system tools are auto-included by agents and must not be exposed over MCP.
/// </summary>
[Fact]
public async Task ListToolsHandler_ExposeAllTools_ReturnsVisibleToolsAndOmitsHidden()
Expand All @@ -48,6 +49,7 @@ public async Task ListToolsHandler_ExposeAllTools_ReturnsVisibleToolsAndOmitsHid

AddLocalTool(services, "search-key", new TestAIFunction("search"));
AddLocalTool(services, "hidden-key", new TestAIFunction("hidden"), hidden: true);
AddLocalTool(services, "system-key", new TestAIFunction("system"), isSystemTool: true);
AddLocalTool(services, "create-key", new TestAIFunction("create"));

using var serviceProvider = services.BuildServiceProvider();
Expand Down Expand Up @@ -348,6 +350,26 @@ await InvokeCallToolHandlerAsync(
TestContext.Current.CancellationToken));
}

/// <summary>
/// Verifies that a system tool cannot be invoked through the call handler even when
/// <c>ExposeAllTools</c> is enabled, because system tools are never exposed over MCP.
/// </summary>
[Fact]
public async Task CallToolHandler_ExposeAll_RejectsSystemTool()
{
var services = CreateServices(configureOptions: options => options.ExposeAllTools = true);

AddLocalTool(services, "system", new TestAIFunction("system"), isSystemTool: true);

using var serviceProvider = services.BuildServiceProvider();

await Assert.ThrowsAsync<McpException>(async () =>
await InvokeCallToolHandlerAsync(
serviceProvider,
"system",
TestContext.Current.CancellationToken));
}

/// <summary>
/// Creates the MCP service collection and registers the CrestApps handlers.
/// </summary>
Expand Down Expand Up @@ -404,13 +426,15 @@ private static void AddToolInstanceSource(IServiceCollection services, string so
/// <param name="registrationName">The keyed registration name.</param>
/// <param name="tool">The local AI function.</param>
/// <param name="hidden">Whether the tool is hidden.</param>
/// <param name="isSystemTool">Whether the tool is a system tool.</param>
private static void AddLocalTool(
IServiceCollection services,
string registrationName,
AIFunction tool,
bool hidden = false)
bool hidden = false,
bool isSystemTool = false)
{
AddLocalToolDefinition(services, registrationName, hidden);
AddLocalToolDefinition(services, registrationName, hidden, isSystemTool);
services.AddKeyedSingleton<AITool>(registrationName, tool);
}

Expand All @@ -420,10 +444,12 @@ private static void AddLocalTool(
/// <param name="services">The service collection.</param>
/// <param name="registrationName">The keyed registration name.</param>
/// <param name="hidden">Whether the tool is hidden.</param>
/// <param name="isSystemTool">Whether the tool is a system tool.</param>
private static void AddLocalToolDefinition(
IServiceCollection services,
string registrationName,
bool hidden = false)
bool hidden = false,
bool isSystemTool = false)
{
services.Configure<AIToolDefinitionOptions>(options =>
{
Expand All @@ -432,6 +458,7 @@ private static void AddLocalToolDefinition(
new AIToolDefinitionEntry(typeof(TestAIFunction))
{
Hidden = hidden,
IsSystemTool = isSystemTool,
});
});
}
Expand Down
Loading