diff --git a/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/ClaudeCommandArgsBuilderFixture.cs b/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/ClaudeCommandArgsBuilderFixture.cs index 812d13101..101b12f0e 100644 --- a/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/ClaudeCommandArgsBuilderFixture.cs +++ b/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/ClaudeCommandArgsBuilderFixture.cs @@ -21,10 +21,19 @@ public void Build_IncludesRequiredFlags() args.Should().Contain("--verbose"); args.Should().Contain("--permission-mode dontAsk"); args.Should().Contain("--no-session-persistence"); - args.Should().Contain("--bare"); args.Should().Contain("--strict-mcp-config"); } + [Test] + public void Build_DoesNotUseBareMode() + { + // --bare (Agent SDK mode) defers HTTP MCP tools behind tool search, which would hide the + // Octopus MCP server now served over loopback HTTP by the credential broker (MD-2096). + var args = MinimalBuilder().Build(); + + args.Should().NotContain("--bare"); + } + [Test] public void Build_IncludesModel_WhenSet() { diff --git a/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpBrokerSmokeTest.cs b/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpBrokerSmokeTest.cs new file mode 100644 index 000000000..28221f84f --- /dev/null +++ b/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpBrokerSmokeTest.cs @@ -0,0 +1,68 @@ +#nullable enable +using System; +using System.Collections.Generic; +using System.Linq; +using System.Threading; +using System.Threading.Tasks; +using Calamari.AiAgent.ClaudeCodeBehaviour; +using Calamari.Testing.Helpers; +using FluentAssertions; +using ModelContextProtocol.Client; +using NUnit.Framework; + +namespace Calamari.AiAgent.Tests.ClaudeCodeBehaviour; + +/// +/// Spike verification (MD-2096): proves the broker mechanism end-to-end without any Octopus token or +/// Claude — it spawns a public stub MCP server as a child, re-exposes it over loopback HTTP, and an +/// HTTP MCP client lists and calls tools through it. Explicit because it runs `npx` (needs Node and a +/// one-time package fetch); not part of normal CI. +/// +[TestFixture] +[Explicit("Spike smoke test: requires Node/npx and network to fetch @modelcontextprotocol/server-everything.")] +[Category("Integration")] +public class McpBrokerSmokeTest +{ + [Test] + public async Task Broker_ProxiesAStdioMcpServer_OverLoopbackHttp() + { + using var cts = new CancellationTokenSource(TimeSpan.FromMinutes(2)); + var log = new InMemoryLog(); + + // The Octopus server is just one spec; here we broker a generic stub MCP server the same way. + var spec = new McpServerSpec + { + Name = "everything", + Command = "npx", + Args = new[] { "-y", "@modelcontextprotocol/server-everything" }, + }; + + await using var broker = await McpBroker.StartAsync(new[] { spec }, log, cts.Token); + + // The agent-facing endpoint is loopback only — the secret-free hop. + var endpoint = broker.Endpoints["everything"]; + endpoint.Host.Should().Be("127.0.0.1"); + + // Connect as the agent would: an HTTP MCP client against the broker endpoint (no secret on this hop). + var clientTransport = new HttpClientTransport(new HttpClientTransportOptions + { + Name = "smoke-test-client", + Endpoint = endpoint, + TransportMode = HttpTransportMode.StreamableHttp, + }); + await using var client = await McpClient.CreateAsync(clientTransport, cancellationToken: cts.Token); + + // tools/list is forwarded from the upstream child through the broker. + var tools = await client.ListToolsAsync(cancellationToken: cts.Token); + tools.Should().Contain(t => t.Name == "echo", "the stub server's tools must be visible through the broker"); + + // tools/call round-trips: the broker forwards the call to the child and returns its result verbatim. + var result = await client.CallToolAsync( + "echo", + new Dictionary { ["message"] = "broker-works" }, + cancellationToken: cts.Token); + + var text = string.Concat(result.Content.OfType().Select(c => c.Text)); + text.Should().Contain("broker-works"); + } +} diff --git a/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpWriterFixture.cs b/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpWriterFixture.cs index f06d8bb2c..17258ab7a 100644 --- a/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpWriterFixture.cs +++ b/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpWriterFixture.cs @@ -1,5 +1,7 @@ +using System; using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text.Json; using Calamari.AiAgent.ClaudeCodeBehaviour; using Calamari.Common.Commands; @@ -29,7 +31,7 @@ public void TearDown() } [Test] - public void SetupMcpConfig_WritesValidJson_WithServers() + public void BuildServerSpecs_IncludesCustomServers() { var vars = new CalamariVariables(); var mcpJson = JsonSerializer.Serialize(new[] @@ -44,165 +46,113 @@ public void SetupMcpConfig_WritesValidJson_WithServers() }); vars.Set(SpecialVariables.Action.Claude.McpServers, mcpJson); - var configPath = new McpWriter(vars).SetupMcpConfig(workingDir); + var specs = new McpWriter(vars).BuildServerSpecs(); - File.Exists(configPath).Should().BeTrue(); - - var json = File.ReadAllText(configPath); - var doc = JsonDocument.Parse(json); - doc.RootElement.TryGetProperty("mcpServers", out var mcpServers).Should().BeTrue(); - mcpServers.TryGetProperty("github", out var github).Should().BeTrue(); - github.GetProperty("command").GetString().Should().Be("npx"); + var github = specs.Single(s => s.Name == "github"); + github.Command.Should().Be("npx"); + github.Args.Should().Equal("-y", "@modelcontextprotocol/server-github"); + github.Env!["TOKEN"].Should().Be("abc123"); } [Test] - public void SetupMcpConfig_WritesEmptyServers_WhenNoneProvided() - { - var configPath = new McpWriter(new CalamariVariables()).SetupMcpConfig(workingDir); - - var json = File.ReadAllText(configPath); - var doc = JsonDocument.Parse(json); - doc.RootElement.TryGetProperty("mcpServers", out var mcpServers).Should().BeTrue(); - mcpServers.EnumerateObject().Should().BeEmpty(); - } - - [Test] - public void GetAllowedTools_ReturnsMcpWildcardPerServer() - { - var vars = new CalamariVariables(); - var mcpJson = JsonSerializer.Serialize(new[] - { - new { name = "github", command = "npx" }, - new { name = "slack", command = "npx" }, - }); - vars.Set(SpecialVariables.Action.Claude.McpServers, mcpJson); - - var tools = new McpWriter(vars).GetAllowedTools(); - - tools.Should().Contain("mcp__github__*"); - tools.Should().Contain("mcp__slack__*"); - } - - [Test] - public void GetAllowedTools_ReturnsEmpty_WhenNoServersConfigured() - { - var tools = new McpWriter(new CalamariVariables()).GetAllowedTools(); - - tools.Should().BeEmpty(); - } - - [Test] - public void SetupMcpConfig_AddsOctopusMcpServer_WhenTokenAndUrlProvided() + public void BuildServerSpecs_IncludesOctopusServer_WhenTokenAndUrlProvided() { var vars = new CalamariVariables(); vars.Set(SpecialVariables.Action.Claude.OctopusToken, "API-TESTKEY"); vars.Set(SpecialVariables.Web.ServerUri, "https://octopus.example.com"); - var configPath = new McpWriter(vars).SetupMcpConfig(workingDir); + var octopus = new McpWriter(vars).BuildServerSpecs().Single(s => s.Name == "octopus"); - var json = File.ReadAllText(configPath); - var doc = JsonDocument.Parse(json); - var mcpServers = doc.RootElement.GetProperty("mcpServers"); - mcpServers.TryGetProperty("octopus", out var octopus).Should().BeTrue(); - octopus.GetProperty("command").GetString().Should().Be("npx"); - - var env = octopus.GetProperty("env"); - env.GetProperty("OCTOPUS_SERVER_URL").GetString().Should().Be("https://octopus.example.com"); - env.GetProperty("OCTOPUS_API_KEY").GetString().Should().Be("API-TESTKEY"); + octopus.Command.Should().Be("npx"); + octopus.Args.Should().Contain("@octopusdeploy/mcp-server"); + // The secret rides in the spec so the broker can hand it to the child process — never to disk. + octopus.Env!["OCTOPUS_SERVER_URL"].Should().Be("https://octopus.example.com"); + octopus.Env!["OCTOPUS_API_KEY"].Should().Be("API-TESTKEY"); } [Test] - public void SetupMcpConfig_SkipsOctopusMcpServer_WhenTokenMissing() + public void BuildServerSpecs_OmitsOctopusServer_WhenTokenMissing() { var vars = new CalamariVariables(); vars.Set(SpecialVariables.Web.ServerUri, "https://octopus.example.com"); - var configPath = new McpWriter(vars).SetupMcpConfig(workingDir); + var specs = new McpWriter(vars).BuildServerSpecs(); - var json = File.ReadAllText(configPath); - var doc = JsonDocument.Parse(json); - doc.RootElement.GetProperty("mcpServers").TryGetProperty("octopus", out _).Should().BeFalse(); + specs.Should().NotContain(s => s.Name == "octopus"); } [Test] - public void SetupMcpConfig_ThrowsOnInvalidMcpJson() + public void BuildServerSpecs_ThrowsOnInvalidMcpJson() { var vars = new CalamariVariables(); vars.Set(SpecialVariables.Action.Claude.McpServers, "not valid json {{{"); - var act = () => new McpWriter(vars).SetupMcpConfig(workingDir); + var act = () => new McpWriter(vars).BuildServerSpecs(); act.Should().Throw().WithMessage("*Failed to parse*"); } [Test] - public void SetupMcpConfig_ThrowsWhenServerMissingName() + public void BuildServerSpecs_ThrowsWhenServerNameBlank() { var vars = new CalamariVariables(); - var mcpJson = JsonSerializer.Serialize(new[] { new { command = "npx" } }); - vars.Set(SpecialVariables.Action.Claude.McpServers, mcpJson); + vars.Set(SpecialVariables.Action.Claude.McpServers, JsonSerializer.Serialize(new[] { new { name = "", command = "npx" } })); - var act = () => new McpWriter(vars).SetupMcpConfig(workingDir); + var act = () => new McpWriter(vars).BuildServerSpecs(); act.Should().Throw().WithMessage("*must have a name*"); } [Test] - public void SetupMcpConfig_ThrowsWhenServerMissingCommand() + public void BuildServerSpecs_ThrowsWhenServerCommandBlank() { var vars = new CalamariVariables(); - var mcpJson = JsonSerializer.Serialize(new[] { new { name = "my-server" } }); - vars.Set(SpecialVariables.Action.Claude.McpServers, mcpJson); + vars.Set(SpecialVariables.Action.Claude.McpServers, JsonSerializer.Serialize(new[] { new { name = "my-server", command = "" } })); - var act = () => new McpWriter(vars).SetupMcpConfig(workingDir); + var act = () => new McpWriter(vars).BuildServerSpecs(); act.Should().Throw().WithMessage("*must have a command*"); } [Test] - public void SetupMcpConfig_InjectsPathEnvVar_WhenNotProvidedByUser() + public void GetAllowedTools_ReturnsMcpWildcardPerServer() { var vars = new CalamariVariables(); - var mcpJson = JsonSerializer.Serialize(new[] + vars.Set(SpecialVariables.Action.Claude.McpServers, JsonSerializer.Serialize(new[] { - new { name = "test-server", command = "node" }, - }); - vars.Set(SpecialVariables.Action.Claude.McpServers, mcpJson); + new { name = "github", command = "npx" }, + new { name = "slack", command = "npx" }, + })); - var configPath = new McpWriter(vars).SetupMcpConfig(workingDir); + var writer = new McpWriter(vars); + var tools = writer.GetAllowedTools(writer.BuildServerSpecs()); - var json = File.ReadAllText(configPath); - var doc = JsonDocument.Parse(json); - var env = doc.RootElement - .GetProperty("mcpServers") - .GetProperty("test-server") - .GetProperty("env"); - env.TryGetProperty("PATH", out _).Should().BeTrue(); + tools.Should().Contain("mcp__github__*"); + tools.Should().Contain("mcp__slack__*"); } [Test] - public void SetupMcpConfig_PreservesUserProvidedPathEnvVar() + public void WriteConfig_WritesSecretFreeHttpEntries() { - var vars = new CalamariVariables(); - var mcpJson = JsonSerializer.Serialize(new[] + // mcp-config.json only references the broker's loopback endpoints — never a command, env, or token. + var endpoints = new Dictionary { - new - { - name = "test-server", - command = "node", - env = new Dictionary { ["PATH"] = "/custom/path" }, - }, - }); - vars.Set(SpecialVariables.Action.Claude.McpServers, mcpJson); + ["octopus"] = new Uri("http://127.0.0.1:54321/"), + ["github"] = new Uri("http://127.0.0.1:54322/"), + }; - var configPath = new McpWriter(vars).SetupMcpConfig(workingDir); + var configPath = new McpWriter(new CalamariVariables()).WriteConfig(workingDir, endpoints); var json = File.ReadAllText(configPath); - var doc = JsonDocument.Parse(json); - var env = doc.RootElement - .GetProperty("mcpServers") - .GetProperty("test-server") - .GetProperty("env"); - env.GetProperty("PATH").GetString().Should().Be("/custom/path"); + var mcpServers = JsonDocument.Parse(json).RootElement.GetProperty("mcpServers"); + + var octopus = mcpServers.GetProperty("octopus"); + octopus.GetProperty("type").GetString().Should().Be("http"); + octopus.GetProperty("url").GetString().Should().Be("http://127.0.0.1:54321/"); + octopus.TryGetProperty("command", out _).Should().BeFalse(); + octopus.TryGetProperty("env", out _).Should().BeFalse(); + mcpServers.GetProperty("github").GetProperty("url").GetString().Should().Be("http://127.0.0.1:54322/"); + + json.Should().NotContain("OCTOPUS_API_KEY"); } } diff --git a/source/Calamari.AiAgent/Calamari.AiAgent.csproj b/source/Calamari.AiAgent/Calamari.AiAgent.csproj index a0479d5e9..c1063b541 100644 --- a/source/Calamari.AiAgent/Calamari.AiAgent.csproj +++ b/source/Calamari.AiAgent/Calamari.AiAgent.csproj @@ -15,6 +15,12 @@ + + + + + diff --git a/source/Calamari.AiAgent/ClaudeCodeBehaviour/ClaudeCodeCliRunner.cs b/source/Calamari.AiAgent/ClaudeCodeBehaviour/ClaudeCodeCliRunner.cs index b81acc0ed..2762ad8f1 100644 --- a/source/Calamari.AiAgent/ClaudeCodeBehaviour/ClaudeCodeCliRunner.cs +++ b/source/Calamari.AiAgent/ClaudeCodeBehaviour/ClaudeCodeCliRunner.cs @@ -3,7 +3,6 @@ using System.Diagnostics; using System.IO; using System.Text; -using System.Text.Json.Serialization; using System.Threading; using System.Threading.Tasks; using Calamari.Common.Plumbing.Logging; @@ -124,27 +123,3 @@ public record UserSkill public required string Name { get; init; } public required string Content { get; init; } } - -public record McpServerConfig -{ - [JsonPropertyName("type")] - public string Type { get; init; } = "stdio"; - - [JsonPropertyName("command")] - public required string Command { get; init; } - - [JsonPropertyName("args")] - public IReadOnlyList? Args { get; init; } - - [JsonPropertyName("env")] - public IReadOnlyDictionary? Env { get; init; } -} - -public record McpServerEntry -{ - public string? Name { get; init; } - public string? Type { get; init; } - public string? Command { get; init; } - public IReadOnlyList? Args { get; init; } - public IReadOnlyDictionary? Env { get; init; } -} diff --git a/source/Calamari.AiAgent/ClaudeCodeBehaviour/ClaudeCommandArgsBuilder.cs b/source/Calamari.AiAgent/ClaudeCodeBehaviour/ClaudeCommandArgsBuilder.cs index 88bd272f1..7f53322ad 100644 --- a/source/Calamari.AiAgent/ClaudeCodeBehaviour/ClaudeCommandArgsBuilder.cs +++ b/source/Calamari.AiAgent/ClaudeCodeBehaviour/ClaudeCommandArgsBuilder.cs @@ -102,7 +102,9 @@ public string Build() args.Append(EscapeArg(model)); } - args.Append(" --bare"); + // --bare (Agent SDK mode) is intentionally NOT used: under --bare, tool search defaults on and + // defers HTTP MCP tools, so the broker's loopback HTTP Octopus server would not load under + // --allowedTools. Standard headless mode loads HTTP MCP tools normally (MD-2096 broker spike). args.Append(" --strict-mcp-config"); args.Append(" --output-format stream-json"); args.Append(" --verbose"); diff --git a/source/Calamari.AiAgent/ClaudeCodeBehaviour/InvokeClaudeCodeBehaviour.cs b/source/Calamari.AiAgent/ClaudeCodeBehaviour/InvokeClaudeCodeBehaviour.cs index 26e5a34a7..afcab5ffc 100644 --- a/source/Calamari.AiAgent/ClaudeCodeBehaviour/InvokeClaudeCodeBehaviour.cs +++ b/source/Calamari.AiAgent/ClaudeCodeBehaviour/InvokeClaudeCodeBehaviour.cs @@ -71,11 +71,19 @@ public async Task Execute(RunningDeployment context) // TODO: THis should be moved up higher in execution Chain. var cancellationToken = new CancellationTokenSource(); + + // Every MCP server runs as a Calamari child behind the credential broker, so no MCP secret + // (the Octopus token or any custom server's secrets) ever reaches Claude Code's environment, + // disk, or descendants. The broker is torn down (terminating each child) on completion, + // cancellation, or throw. var mcpWriter = new McpWriter(variables); - var mcpConfig = mcpWriter.SetupMcpConfig(workingDir); + var serverSpecs = mcpWriter.BuildServerSpecs(); + await using var mcpBroker = await McpBroker.StartAsync(serverSpecs, log, cancellationToken.Token); + + var mcpConfig = mcpWriter.WriteConfig(workingDir, mcpBroker.Endpoints); - var allowedTools = AllowedTools(variables); - allowedTools.AddRange(mcpWriter.GetAllowedTools()); + var allowedTools = new List(AllowedTools(variables)); + allowedTools.AddRange(mcpWriter.GetAllowedTools(serverSpecs)); argsBuilder = argsBuilder.WithAllowedTools(allowedTools); new SkillsWriter(variables).SetupSkills(workingDir); diff --git a/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpBroker.cs b/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpBroker.cs new file mode 100644 index 000000000..56af00aa2 --- /dev/null +++ b/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpBroker.cs @@ -0,0 +1,217 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Net; +using System.Threading; +using System.Threading.Tasks; +using Calamari.Common.Plumbing.Logging; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Hosting; +using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol.Client; +using ModelContextProtocol.Protocol; + +namespace Calamari.AiAgent.ClaudeCodeBehaviour; + +/// +/// An MCP server for the broker to front: the command Calamari spawns and the secrets it holds. +/// This is also the shape of each entry in the user's McpServers variable, deserialized directly. +/// +public record McpServerSpec +{ + public required string Name { get; init; } + public required string Command { get; init; } + public IReadOnlyList? Args { get; init; } + public IReadOnlyDictionary? Env { get; init; } +} + +/// +/// In-process credential broker for MCP servers (MD-2096 spike). +/// +/// Calamari spawns each MCP server as its OWN child — the server's secrets live only in that child's +/// environment — and re-exposes it to the agent over a secret-free loopback HTTP endpoint. No MCP +/// secret ever enters Claude Code's environment, disk, or descendants; the agent reaches each server +/// sideways through the broker, which is a transparent proxy: +/// upstream = an MCP client over stdio that holds the real secrets; +/// downstream = an MCP server over loopback HTTP that forwards tools/list and tools/call. +/// +/// One loopback host is started per brokered server (the count is small for a deployment step); the +/// two forwarding handlers are the entire proxy and the audit chokepoint. +/// +public sealed class McpBroker : IAsyncDisposable +{ + readonly IReadOnlyList servers; + + McpBroker(IReadOnlyList servers) + { + this.servers = servers; + Endpoints = servers.ToDictionary(server => server.Name, server => server.Endpoint); + } + + /// The loopback URL for each brokered server, keyed by server name (the agent's mcp-config entries). + public IReadOnlyDictionary Endpoints { get; } + + public static async Task StartAsync(IReadOnlyList specs, ILog log, CancellationToken cancellationToken) + { + var started = new List(); + try + { + foreach (var spec in specs) + started.Add(await BrokeredServer.StartAsync(spec, log, cancellationToken)); + + return new McpBroker(started); + } + catch + { + // Tear down anything already started so a failure part-way through leaves no orphaned child. + for (var i = started.Count - 1; i >= 0; i--) + await started[i].DisposeAsync(); + throw; + } + } + + public async ValueTask DisposeAsync() + { + for (var i = servers.Count - 1; i >= 0; i--) + await servers[i].DisposeAsync(); + } + + /// One brokered server: its upstream stdio client and its downstream loopback HTTP host. + sealed class BrokeredServer : IAsyncDisposable + { + readonly McpClient upstream; + readonly WebApplication app; + + BrokeredServer(string name, McpClient upstream, WebApplication app, Uri endpoint) + { + Name = name; + this.upstream = upstream; + this.app = app; + Endpoint = endpoint; + } + + public string Name { get; } + public Uri Endpoint { get; } + + public static async Task StartAsync(McpServerSpec spec, ILog log, CancellationToken cancellationToken) + { + // Upstream: spawn the MCP server as a Calamari child holding its secrets. We don't inherit + // the parent environment — the secrets are the point of the broker — so the child gets only + // the spec's env plus the minimal non-secret system variables it needs to launch. + var transport = new StdioClientTransport(new StdioClientTransportOptions + { + Name = $"{spec.Name}-upstream", + Command = spec.Command, + Arguments = spec.Args?.ToArray(), + InheritEnvironmentVariables = false, + EnvironmentVariables = BuildChildEnvironment(spec.Env), + StandardErrorLines = line => log.Verbose($"[mcp:{spec.Name}] {line}"), + }); + + var upstream = await McpClient.CreateAsync(transport, cancellationToken: cancellationToken); + + try + { + // Readiness/health check: one round-trip proves the child started and accepted its + // secrets (covers process cold-start and any non-empty-credential startup requirement). + await upstream.ListToolsAsync(cancellationToken: cancellationToken); + + // Downstream: loopback HTTP MCP server on an OS-assigned ephemeral port. The two handlers + // forward verbatim to the upstream client and are the audit chokepoint. + var builder = WebApplication.CreateSlimBuilder(); + // Bind to an OS-assigned ephemeral port on the IPv4 loopback. ListenLocalhost does not + // support dynamic ports, so bind the loopback address explicitly. + builder.WebHost.ConfigureKestrel(kestrel => kestrel.Listen(IPAddress.Loopback, 0)); + + builder.Services + .AddMcpServer() + .WithHttpTransport() + .WithListToolsHandler(async (_, ct) => + { + var tools = await upstream.ListToolsAsync(cancellationToken: ct); + // Forward the upstream protocol tools verbatim (preserves names + input schemas). + return new ListToolsResult { Tools = tools.Select(tool => tool.ProtocolTool).ToList() }; + }) + .WithCallToolHandler(async (callContext, ct) => + { + var request = callContext.Params!; + log.Verbose($"MCP tool call [{spec.Name}]: {request.Name}"); // audit point + var arguments = request.Arguments?.ToDictionary(arg => arg.Key, arg => (object?)arg.Value); + // CallToolResult is the same type on both sides — pass the upstream result through. + return await upstream.CallToolAsync(request.Name, arguments, cancellationToken: ct); + }); + + var app = builder.Build(); + app.MapMcp(); + await app.StartAsync(cancellationToken); + + var endpoint = ResolveLoopbackEndpoint(app); + log.Verbose($"MCP broker for '{spec.Name}' listening on {endpoint}"); + return new BrokeredServer(spec.Name, upstream, app, endpoint); + } + catch + { + await upstream.DisposeAsync(); + throw; + } + } + + static Dictionary BuildChildEnvironment(IReadOnlyDictionary? specEnv) + { + var environment = new Dictionary(); + if (specEnv != null) + { + foreach (var kvp in specEnv) + environment[kvp.Key] = kvp.Value; + } + + // Because we don't inherit the parent environment, hand the child the minimal non-secret + // system variables a launcher (e.g. npx/node) needs to start and resolve its cache cross-platform, + // plus the HTTP proxy routing variables a proxied worker needs to reach the npm registry and the + // Octopus Server (the Octopus MCP server's HTTP client honours these). These are routing config, + // not secrets. + string[] passThrough = + { + "PATH", "Path", "HOME", "USERPROFILE", "APPDATA", "LOCALAPPDATA", + "SystemRoot", "windir", "TEMP", "TMP", "PATHEXT", "ComSpec", + "ProgramFiles", "ProgramFiles(x86)", + "HTTP_PROXY", "HTTPS_PROXY", "NO_PROXY", + "http_proxy", "https_proxy", "no_proxy", + }; + foreach (var name in passThrough) + { + var value = Environment.GetEnvironmentVariable(name); + if (value != null && !environment.ContainsKey(name)) + environment[name] = value; + } + + return environment; + } + + static Uri ResolveLoopbackEndpoint(WebApplication app) + { + var address = app.Urls.FirstOrDefault() + ?? throw new InvalidOperationException("MCP broker did not bind to any address."); + // Normalise to an explicit IPv4 loopback URL regardless of how Kestrel reports the bound address. + var bound = new Uri(address); + return new UriBuilder(Uri.UriSchemeHttp, "127.0.0.1", bound.Port).Uri; + } + + public async ValueTask DisposeAsync() + { + // Stop the listener first, then terminate the upstream stdio child (kills the MCP server process). + try + { + using var stopTimeout = new CancellationTokenSource(TimeSpan.FromSeconds(5)); + await app.StopAsync(stopTimeout.Token); + } + catch + { + // Best-effort shutdown — disposal must not mask the original outcome of the run. + } + + await app.DisposeAsync(); + await upstream.DisposeAsync(); + } + } +} diff --git a/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpWriter.cs b/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpWriter.cs index 6976b8b50..309b45fc2 100644 --- a/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpWriter.cs +++ b/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpWriter.cs @@ -3,19 +3,49 @@ using System.IO; using System.Linq; using System.Text.Json; +using System.Text.Json.Serialization; using Calamari.Common.Commands; using Calamari.Common.Plumbing.Logging; using Calamari.Common.Plumbing.Variables; namespace Calamari.AiAgent.ClaudeCodeBehaviour; +/// +/// Prepares MCP configuration for a Claude Code run. Every MCP server (the Octopus server and any +/// user-configured custom servers) now runs as a Calamari child behind the credential broker +/// (see ), so this no longer writes any secret to disk. It parses the +/// configured servers into s for the broker to spawn, and writes a +/// secret-free mcp-config.json that points Claude Code at the broker's loopback HTTP endpoints. +/// public class McpWriter(IVariables variables) { - static readonly string ConfigName = "mcp-config.json"; - - public string SetupMcpConfig(string workingDir) + const string ConfigName = "mcp-config.json"; + + /// + /// The set of MCP servers to front: the Octopus server (when a token + URL are available) plus any + /// user-configured custom servers. The broker spawns each as a Calamari child holding its secrets. + /// + public IReadOnlyList BuildServerSpecs() { - var mcpServers = BuildMcpServers(); + var specs = new List(); + specs.AddRange(BuildCustomServerSpecs()); + + var octopus = BuildOctopusServerSpec(); + if (octopus != null) + specs.Add(octopus); + + return specs; + } + + /// + /// Writes a secret-free mcp-config.json mapping each brokered server to its loopback HTTP endpoint. + /// + public string WriteConfig(string workingDir, IReadOnlyDictionary brokerEndpoints) + { + var mcpServers = brokerEndpoints.ToDictionary( + endpoint => endpoint.Key, + endpoint => new McpConfigEntry { Url = endpoint.Value.ToString() }); + var config = new { mcpServers }; var json = JsonSerializer.Serialize(config, new JsonSerializerOptions { WriteIndented = true }); var path = Path.Combine(workingDir, ConfigName); @@ -23,109 +53,78 @@ public string SetupMcpConfig(string workingDir) return path; } - public IEnumerable GetAllowedTools() - { - var mcpServers = GetCustomMcpServers(); - - // TODO: Use explicitly allowed MCP tools - return mcpServers.Select(serverName => $"mcp__{serverName.Name}__*"); - } - - Dictionary BuildMcpServers() - { - var path = Environment.GetEnvironmentVariable("PATH") ?? ""; - - var servers = AddCustomMcpServer(path); - AddOctopusMcp(servers, path); - return servers; - } + // TODO: Use explicitly allowed MCP tools rather than a per-server wildcard. + public IEnumerable GetAllowedTools(IReadOnlyList specs) + => specs.Select(spec => $"mcp__{spec.Name}__*"); - void AddOctopusMcp(Dictionary servers, string path) + McpServerSpec? BuildOctopusServerSpec() { var octopusToken = variables.Get(SpecialVariables.Action.Claude.OctopusToken); if (string.IsNullOrWhiteSpace(octopusToken)) - return; - + return null; - // Octopus MCP server is always added when a token is available var octopusServerUrl = variables.Get(SpecialVariables.Web.ServerUri); if (string.IsNullOrWhiteSpace(octopusServerUrl)) { - Log.Warn("Unable to find Octopus Server URL"); + Log.Warn("Unable to find Octopus Server URL; the Octopus MCP server will not be available."); + return null; } - else + + Log.Verbose("Octopus Server URL: " + octopusServerUrl); + return new McpServerSpec { - Log.Verbose("Octopus Server URL: " + octopusServerUrl); - servers["octopus"] = new McpServerConfig + Name = "octopus", + Command = "npx", + Args = new[] { "-y", "@octopusdeploy/mcp-server" }, + Env = new Dictionary { - Command = "npx", - Args = new[] { "-y", "@octopusdeploy/mcp-server" }, - Env = new Dictionary - { - ["OCTOPUS_SERVER_URL"] = octopusServerUrl, - ["OCTOPUS_API_KEY"] = octopusToken, - ["PATH"] = path, - }, - }; - } + ["OCTOPUS_SERVER_URL"] = octopusServerUrl, + ["OCTOPUS_API_KEY"] = octopusToken, + }, + }; } - List GetCustomMcpServers() + // The McpServers variable is just a list of server specs the user declares; deserialize straight + // into McpServerSpec and validate, rather than maintaining a separate near-identical parse type. + IReadOnlyList BuildCustomServerSpecs() { var mcpServersJson = variables.Get(SpecialVariables.Action.Claude.McpServers); if (string.IsNullOrWhiteSpace(mcpServersJson)) - { - return new List(); - } - + return Array.Empty(); + + List? servers; try { - var customServers = JsonSerializer.Deserialize>(mcpServersJson, new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); - - return customServers ?? new List(); + servers = JsonSerializer.Deserialize>(mcpServersJson, new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); } catch (JsonException ex) { throw new CommandException($"Failed to parse MCP servers configuration: {ex.Message}"); } - } - Dictionary AddCustomMcpServer(string path) - { - var entries = GetCustomMcpServers(); + if (servers == null) + return Array.Empty(); - var mcpServerConfigs = new Dictionary(); - if (entries.Any()) + foreach (var server in servers) { - foreach (var entry in entries) - { - if (string.IsNullOrWhiteSpace(entry.Name)) - throw new CommandException("Each MCP server must have a name."); - if (string.IsNullOrWhiteSpace(entry.Command)) - throw new CommandException($"MCP server '{entry.Name}' must have a command."); - - var env = entry.Env != null - ? new Dictionary(entry.Env) - : new Dictionary(); - - if (!env.ContainsKey("PATH")) - env["PATH"] = path; - - mcpServerConfigs[entry.Name] = new McpServerConfig - { - Type = entry.Type ?? "stdio", - Command = entry.Command, - Args = entry.Args, - Env = env, - }; - Log.Verbose($"MCP server '{entry.Name}' added."); - } + if (string.IsNullOrWhiteSpace(server.Name)) + throw new CommandException("Each MCP server must have a name."); + if (string.IsNullOrWhiteSpace(server.Command)) + throw new CommandException($"MCP server '{server.Name}' must have a command."); + + Log.Verbose($"MCP server '{server.Name}' added."); } - return mcpServerConfigs; + return servers; } +} +/// One entry written to the agent's mcp-config.json — always the broker's loopback HTTP endpoint. +file record McpConfigEntry +{ + [JsonPropertyName("type")] + public string Type { get; init; } = "http"; - - -} \ No newline at end of file + [JsonPropertyName("url")] + public required string Url { get; init; } +}