diff --git a/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpWriterFixture.cs b/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpWriterFixture.cs index f06d8bb2c..81a768262 100644 --- a/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpWriterFixture.cs +++ b/source/Calamari.AiAgent.Tests/ClaudeCodeBehaviour/McpWriterFixture.cs @@ -44,23 +44,30 @@ public void SetupMcpConfig_WritesValidJson_WithServers() }); vars.Set(SpecialVariables.Action.Claude.McpServers, mcpJson); - var configPath = new McpWriter(vars).SetupMcpConfig(workingDir); + var result = new McpWriter(vars).SetupMcpConfig(workingDir); - File.Exists(configPath).Should().BeTrue(); + File.Exists(result.Path).Should().BeTrue(); - var json = File.ReadAllText(configPath); + var json = File.ReadAllText(result.Path); 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"); + + // The secret env value is referenced as a ${VAR} placeholder on disk, with the real + // value surfaced for injection into the claude process env. + var tokenPlaceholder = github.GetProperty("env").GetProperty("TOKEN").GetString(); + tokenPlaceholder.Should().Be("${MCP_GITHUB_TOKEN}"); + result.SecretEnvVars["MCP_GITHUB_TOKEN"].Should().Be("abc123"); + json.Should().NotContain("abc123"); } [Test] public void SetupMcpConfig_WritesEmptyServers_WhenNoneProvided() { - var configPath = new McpWriter(new CalamariVariables()).SetupMcpConfig(workingDir); + var result = new McpWriter(new CalamariVariables()).SetupMcpConfig(workingDir); - var json = File.ReadAllText(configPath); + var json = File.ReadAllText(result.Path); var doc = JsonDocument.Parse(json); doc.RootElement.TryGetProperty("mcpServers", out var mcpServers).Should().BeTrue(); mcpServers.EnumerateObject().Should().BeEmpty(); @@ -98,9 +105,9 @@ public void SetupMcpConfig_AddsOctopusMcpServer_WhenTokenAndUrlProvided() 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 result = new McpWriter(vars).SetupMcpConfig(workingDir); - var json = File.ReadAllText(configPath); + var json = File.ReadAllText(result.Path); var doc = JsonDocument.Parse(json); var mcpServers = doc.RootElement.GetProperty("mcpServers"); mcpServers.TryGetProperty("octopus", out var octopus).Should().BeTrue(); @@ -108,7 +115,12 @@ public void SetupMcpConfig_AddsOctopusMcpServer_WhenTokenAndUrlProvided() 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"); + + // The Octopus token is referenced as a ${VAR} placeholder on disk, with the real value + // surfaced for injection into the claude process env. + env.GetProperty("OCTOPUS_API_KEY").GetString().Should().Be("${OCTOPUS_API_KEY}"); + result.SecretEnvVars["OCTOPUS_API_KEY"].Should().Be("API-TESTKEY"); + json.Should().NotContain("API-TESTKEY"); } [Test] @@ -117,9 +129,9 @@ public void SetupMcpConfig_SkipsOctopusMcpServer_WhenTokenMissing() var vars = new CalamariVariables(); vars.Set(SpecialVariables.Web.ServerUri, "https://octopus.example.com"); - var configPath = new McpWriter(vars).SetupMcpConfig(workingDir); + var result = new McpWriter(vars).SetupMcpConfig(workingDir); - var json = File.ReadAllText(configPath); + var json = File.ReadAllText(result.Path); var doc = JsonDocument.Parse(json); doc.RootElement.GetProperty("mcpServers").TryGetProperty("octopus", out _).Should().BeFalse(); } @@ -169,9 +181,9 @@ public void SetupMcpConfig_InjectsPathEnvVar_WhenNotProvidedByUser() }); vars.Set(SpecialVariables.Action.Claude.McpServers, mcpJson); - var configPath = new McpWriter(vars).SetupMcpConfig(workingDir); + var result = new McpWriter(vars).SetupMcpConfig(workingDir); - var json = File.ReadAllText(configPath); + var json = File.ReadAllText(result.Path); var doc = JsonDocument.Parse(json); var env = doc.RootElement .GetProperty("mcpServers") @@ -195,9 +207,9 @@ public void SetupMcpConfig_PreservesUserProvidedPathEnvVar() }); vars.Set(SpecialVariables.Action.Claude.McpServers, mcpJson); - var configPath = new McpWriter(vars).SetupMcpConfig(workingDir); + var result = new McpWriter(vars).SetupMcpConfig(workingDir); - var json = File.ReadAllText(configPath); + var json = File.ReadAllText(result.Path); var doc = JsonDocument.Parse(json); var env = doc.RootElement .GetProperty("mcpServers") diff --git a/source/Calamari.AiAgent/ClaudeCodeBehaviour/InvokeClaudeCodeBehaviour.cs b/source/Calamari.AiAgent/ClaudeCodeBehaviour/InvokeClaudeCodeBehaviour.cs index 26e5a34a7..9758b0b6b 100644 --- a/source/Calamari.AiAgent/ClaudeCodeBehaviour/InvokeClaudeCodeBehaviour.cs +++ b/source/Calamari.AiAgent/ClaudeCodeBehaviour/InvokeClaudeCodeBehaviour.cs @@ -104,16 +104,23 @@ public async Task Execute(RunningDeployment context) } argsBuilder.WithSystemPromptFile(new SystemPromptWriter().WriteSystemPromptFile(workingDir)); - argsBuilder.WithMcpConfigPath(mcpConfig); + argsBuilder.WithMcpConfigPath(mcpConfig.Path); + + // MCP secrets are referenced as ${VAR} in mcp-config.json and supplied to the claude + // process env here, so they aren't written to disk in plaintext. Claude expands them + // into each stdio server's env block at launch. + var alwaysSet = new Dictionary + { + ["ANTHROPIC_API_KEY"] = apiToken, + ["CLAUDE_CODE_SUBPROCESS_ENV_SCRUB"] = "1", // Strips Anthropic/cloud credentials from Bash, hook, and MCP subprocess environments + }; + foreach (var kvp in mcpConfig.SecretEnvVars) + alwaysSet[kvp.Key] = kvp.Value; var environment = ClaudeCodeEnvironment.Build( ClaudeCodeEnvironment.GetCurrentEnvironmentVariables(), PassThroughEnvironmentVariables(variables), - new Dictionary - { - ["ANTHROPIC_API_KEY"] = apiToken, - ["CLAUDE_CODE_SUBPROCESS_ENV_SCRUB"] = "1", // Strips Anthropic/cloud credentials from Bash, hook, and MCP subprocess environments - }); + alwaysSet); var response = await new ClaudeCodeCliRunner(log).RunAsync( argsBuilder, diff --git a/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpWriter.cs b/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpWriter.cs index 6976b8b50..4d64b567c 100644 --- a/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpWriter.cs +++ b/source/Calamari.AiAgent/ClaudeCodeBehaviour/McpWriter.cs @@ -13,14 +13,18 @@ public class McpWriter(IVariables variables) { static readonly string ConfigName = "mcp-config.json"; - public string SetupMcpConfig(string workingDir) + public McpConfigResult SetupMcpConfig(string workingDir) { - var mcpServers = BuildMcpServers(); + // Secret env values are referenced from mcp-config.json as ${VAR} placeholders and + // passed to the claude process env instead of being written to disk in plaintext. + // Claude expands ${VAR} in stdio server env values from its own process env at launch. + var secretEnvVars = new Dictionary(); + var mcpServers = BuildMcpServers(secretEnvVars); var config = new { mcpServers }; var json = JsonSerializer.Serialize(config, new JsonSerializerOptions { WriteIndented = true }); var path = Path.Combine(workingDir, ConfigName); File.WriteAllText(path, json); - return path; + return new McpConfigResult(path, secretEnvVars); } public IEnumerable GetAllowedTools() @@ -31,16 +35,16 @@ public IEnumerable GetAllowedTools() return mcpServers.Select(serverName => $"mcp__{serverName.Name}__*"); } - Dictionary BuildMcpServers() + Dictionary BuildMcpServers(Dictionary secretEnvVars) { var path = Environment.GetEnvironmentVariable("PATH") ?? ""; - - var servers = AddCustomMcpServer(path); - AddOctopusMcp(servers, path); + + var servers = AddCustomMcpServer(path, secretEnvVars); + AddOctopusMcp(servers, path, secretEnvVars); return servers; } - void AddOctopusMcp(Dictionary servers, string path) + void AddOctopusMcp(Dictionary servers, string path, Dictionary secretEnvVars) { var octopusToken = variables.Get(SpecialVariables.Action.Claude.OctopusToken); if (string.IsNullOrWhiteSpace(octopusToken)) @@ -56,6 +60,7 @@ void AddOctopusMcp(Dictionary servers, string path) else { Log.Verbose("Octopus Server URL: " + octopusServerUrl); + secretEnvVars["OCTOPUS_API_KEY"] = octopusToken; servers["octopus"] = new McpServerConfig { Command = "npx", @@ -63,7 +68,7 @@ void AddOctopusMcp(Dictionary servers, string path) Env = new Dictionary { ["OCTOPUS_SERVER_URL"] = octopusServerUrl, - ["OCTOPUS_API_KEY"] = octopusToken, + ["OCTOPUS_API_KEY"] = "${OCTOPUS_API_KEY}", ["PATH"] = path, }, }; @@ -90,7 +95,7 @@ List GetCustomMcpServers() } } - Dictionary AddCustomMcpServer(string path) + Dictionary AddCustomMcpServer(string path, Dictionary secretEnvVars) { var entries = GetCustomMcpServers(); @@ -104,9 +109,25 @@ Dictionary AddCustomMcpServer(string path) 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(); + var env = new Dictionary(); + if (entry.Env != null) + { + foreach (var kvp in entry.Env) + { + // PATH is not a secret and is needed verbatim to locate the server command. + if (kvp.Key == "PATH") + { + env[kvp.Key] = kvp.Value; + continue; + } + + // Custom server env values may be secrets, so reference them as ${VAR} + // placeholders and pass the real values via the claude process env. + var placeholder = ReserveEnvVarName(secretEnvVars, entry.Name, kvp.Key); + secretEnvVars[placeholder] = kvp.Value; + env[kvp.Key] = $"${{{placeholder}}}"; + } + } if (!env.ContainsKey("PATH")) env["PATH"] = path; @@ -125,7 +146,22 @@ Dictionary AddCustomMcpServer(string path) return mcpServerConfigs; } + // Builds a deterministic, collision-safe env var name for a custom server's env entry. + static string ReserveEnvVarName(Dictionary secretEnvVars, string serverName, string key) + { + var raw = $"MCP_{serverName}_{key}".ToUpperInvariant(); + var chars = raw.Select(c => char.IsLetterOrDigit(c) ? c : '_').ToArray(); + var name = new string(chars); + if (name.Length == 0 || char.IsDigit(name[0])) + name = "_" + name; + + var candidate = name; + var suffix = 1; + while (secretEnvVars.ContainsKey(candidate)) + candidate = $"{name}_{suffix++}"; + + return candidate; + } +} - - -} \ No newline at end of file +public record McpConfigResult(string Path, IReadOnlyDictionary SecretEnvVars);