Skip to content

Commit 82089ca

Browse files
gfraiteurclaude
andcommitted
Improve MCP server UX and security model
- Remove auto-reject for high-risk commands; always require manual approval - Stream command output to console in real-time instead of buffering - Use temp file for PowerShell execution to avoid escaping issues - Clarify env var risk: OUTPUT to container is dangerous, internal USE is safe - Remove redundant MCP docs from README 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]>
1 parent 2e295f5 commit 82089ca

File tree

4 files changed

+75
-83
lines changed

4 files changed

+75
-83
lines changed

README.md

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -553,62 +553,7 @@ Common environment variables (see [`EnvironmentVariableNames.cs`](src/PostSharp.
553553
| `AZURE_DEVOPS_TOKEN` | Azure DevOps integration |
554554
| `TYPESENSE_API_KEY` | Search indexing |
555555

556-
## MCP Approval Server (Docker Support)
557556

558-
When running Claude Code inside Docker containers, certain operations require host-level access (git push, GitHub CLI, etc.). The MCP Approval Server provides a secure, human-in-the-loop workflow for these operations.
559-
560-
### Architecture
561-
562-
```
563-
┌─────────────────────────────────────────┐
564-
│ Docker Container │
565-
│ ┌─────────────┐ │
566-
│ │ Claude Code │──▶ MCP Client │
567-
│ └─────────────┘ (execute_command) │
568-
└──────────────────────┬──────────────────┘
569-
│ HTTP/SSE
570-
571-
┌─────────────────────────────────────────┐
572-
│ Host: MCP Approval Server │
573-
│ 1. Receive request │
574-
│ 2. AI risk analysis (Claude CLI) │
575-
│ 3. Auto-approve/reject or prompt user │
576-
│ 4. Execute if approved │
577-
│ 5. Return result │
578-
└─────────────────────────────────────────┘
579-
```
580-
581-
### Features
582-
583-
- **AI Risk Analysis**: Each command is analyzed by Claude CLI for risk assessment
584-
- **Auto-approve**: LOW risk commands with AI APPROVE recommendation
585-
- **Auto-reject**: HIGH/CRITICAL risk commands with AI REJECT recommendation
586-
- **Git Push Analysis**: Automatically fetches and analyzes commit diffs for secrets, credentials, and inappropriate content
587-
- **Session Tracking**: Maintains command history to detect suspicious patterns
588-
- **Attack Detection**: Detects unicode homoglyphs, shell injection, path traversal, and other evasion techniques
589-
590-
### Usage
591-
592-
The MCP server starts automatically with `DockerBuild.ps1 -Claude`. To disable:
593-
594-
```powershell
595-
.\DockerBuild.ps1 -Claude -NoMcp
596-
```
597-
598-
Inside the container, privileged commands are routed through the MCP server automatically via the `host-approval` MCP configuration.
599-
600-
### Supported Operations
601-
602-
| Operation | Risk Level |
603-
|-----------|------------|
604-
| `gh pr view` | LOW |
605-
| `gh pr create` | LOW |
606-
| `git push` (feature branch) | LOW |
607-
| `git push` (main/develop) | MEDIUM |
608-
| `gh pr merge` | MEDIUM |
609-
| `git push --force` | HIGH |
610-
| `dotnet nuget push` | HIGH |
611-
| Secret exfiltration attempts | CRITICAL |
612557

613558
## Documentation
614559

src/PostSharp.Engineering.BuildTools/Mcp/Services/ApprovalPrompter.cs

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -55,17 +55,7 @@ public Task<bool> RequestApprovalAsync(
5555
return Task.FromResult( true );
5656
}
5757

58-
// Auto-reject HIGH/CRITICAL risk commands when AI recommends rejection
59-
if ( ( assessment.Level == RiskLevel.High || assessment.Level == RiskLevel.Critical )
60-
&& assessment.Recommendation == Recommendation.Reject )
61-
{
62-
AnsiConsole.WriteLine();
63-
AnsiConsole.MarkupLine( $"[red]Auto-rejected ({assessment.Level.ToString().ToUpperInvariant()} risk):[/] [white]{Markup.Escape( command )}[/]" );
64-
AnsiConsole.MarkupLine( $"[dim]Reason: {Markup.Escape( assessment.Reason )}[/]" );
65-
AnsiConsole.WriteLine();
66-
67-
return Task.FromResult( false );
68-
}
58+
// HIGH/CRITICAL risk commands always require manual approval (no auto-reject)
6959

7060
AnsiConsole.WriteLine();
7161
AnsiConsole.Write( new Rule( "[yellow]Command Approval Request[/]" ) );

src/PostSharp.Engineering.BuildTools/Mcp/Services/CommandExecutor.cs

Lines changed: 61 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,11 @@
11
// Copyright (c) SharpCrafters s.r.o. See the LICENSE.md file in the root directory of this repository root for details.
22

33
using PostSharp.Engineering.BuildTools.Mcp.Models;
4+
using Spectre.Console;
45
using System;
56
using System.Diagnostics;
7+
using System.IO;
8+
using System.Text;
69
using System.Threading;
710
using System.Threading.Tasks;
811

@@ -21,12 +24,17 @@ public async Task<CommandResult> ExecuteAsync(
2124
CancellationToken cancellationToken = default )
2225
#pragma warning restore CA1822
2326
{
27+
// Write the command to a temp file to avoid escaping issues
28+
var tempFile = Path.Combine( Path.GetTempPath(), $"mcp-cmd-{Guid.NewGuid():N}.ps1" );
29+
2430
try
2531
{
32+
await File.WriteAllTextAsync( tempFile, command, Encoding.UTF8, cancellationToken );
33+
2634
var startInfo = new ProcessStartInfo
2735
{
2836
FileName = "pwsh",
29-
Arguments = $"-NoProfile -Command \"{EscapeForPowerShell( command )}\"",
37+
Arguments = $"-NoProfile -File \"{tempFile}\"",
3038
WorkingDirectory = workingDirectory,
3139
RedirectStandardOutput = true,
3240
RedirectStandardError = true,
@@ -41,24 +49,67 @@ public async Task<CommandResult> ExecuteAsync(
4149
return CommandResult.Error( "Failed to start PowerShell process" );
4250
}
4351

44-
var stdout = await process.StandardOutput.ReadToEndAsync( cancellationToken );
45-
var stderr = await process.StandardError.ReadToEndAsync( cancellationToken );
52+
var stdoutBuilder = new StringBuilder();
53+
var stderrBuilder = new StringBuilder();
54+
55+
// Stream output to console in real-time
56+
var stdoutTask = ReadStreamAsync( process.StandardOutput, stdoutBuilder, "stdout", cancellationToken );
57+
var stderrTask = ReadStreamAsync( process.StandardError, stderrBuilder, "stderr", cancellationToken );
58+
59+
await Task.WhenAll( stdoutTask, stderrTask );
4660
await process.WaitForExitAsync( cancellationToken );
4761

48-
return CommandResult.Success( stdout, stderr, process.ExitCode );
62+
return CommandResult.Success( stdoutBuilder.ToString(), stderrBuilder.ToString(), process.ExitCode );
4963
}
5064
catch ( Exception ex )
5165
{
5266
return CommandResult.Error( $"Execution error: {ex.Message}" );
5367
}
68+
finally
69+
{
70+
// Clean up temp file
71+
try
72+
{
73+
if ( File.Exists( tempFile ) )
74+
{
75+
File.Delete( tempFile );
76+
}
77+
}
78+
catch
79+
{
80+
// Ignore cleanup errors
81+
}
82+
}
5483
}
5584

56-
private static string EscapeForPowerShell( string command )
85+
private static async Task ReadStreamAsync(
86+
StreamReader reader,
87+
StringBuilder output,
88+
string streamName,
89+
CancellationToken cancellationToken )
5790
{
58-
// Escape double quotes for PowerShell command line
59-
// Using backtick (`) as the PowerShell escape character
60-
return command
61-
.Replace( "\"", "`\"", StringComparison.Ordinal )
62-
.Replace( "$", "`$", StringComparison.Ordinal );
91+
var isStderr = streamName == "stderr";
92+
93+
while ( !cancellationToken.IsCancellationRequested )
94+
{
95+
var line = await reader.ReadLineAsync( cancellationToken );
96+
97+
if ( line == null )
98+
{
99+
break;
100+
}
101+
102+
output.AppendLine( line );
103+
104+
// Display to console with appropriate formatting
105+
if ( isStderr )
106+
{
107+
AnsiConsole.MarkupLine( $"[red]{line.EscapeMarkup()}[/]" );
108+
}
109+
else
110+
{
111+
AnsiConsole.WriteLine( line );
112+
}
113+
}
63114
}
64115
}

src/PostSharp.Engineering.BuildTools/Mcp/Services/RiskAnalyzer.cs

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -73,9 +73,13 @@ public sealed class RiskAnalyzer
7373
- Sequences that seem to be probing or gathering information before a bigger action
7474
- Mismatches between claimed purpose and actual command behavior
7575
76-
### Environment variables
77-
- Read ANY environment variable: HIGH risk, expose secrets
78-
- Write ANY environment variable: HIGH risk
76+
### Environment Variables (Leakage Risk)
77+
The MCP server runs on the host with access to secrets. The requesting Claude runs in Docker without them.
78+
The risk is LEAKING secrets to the container, not using them for legitimate operations.
79+
- Commands that USE env vars internally (e.g., `git push` using GITHUB_TOKEN): LOW risk - secrets stay on host
80+
- Commands that OUTPUT env vars (e.g., `echo $VAR`, `printenv`, `env`, `Get-ChildItem Env:`): CRITICAL risk - leaks to container
81+
- Commands that write env vars to files: CRITICAL risk - files may be accessible to container
82+
- Piping env var values anywhere the container can read: CRITICAL risk
7983
8084
### Content Appropriateness (GitHub comments, issues, PRs)
8185
- Any insulting, offensive, or inappropriate language: HIGH risk
@@ -120,10 +124,12 @@ public sealed class RiskAnalyzer
120124
- If purpose and command don't align perfectly = MEDIUM risk minimum
121125
122126
### Environment/Secret Exfiltration
123-
- `echo $VAR`, `printenv`, `env`, `set` = HIGH risk (exposes secrets)
124-
- Writing environment variables to files
125-
- Commands that could leak secrets in output or error messages
126-
- Piping sensitive data anywhere = HIGH risk
127+
Remember: The container Claude will receive command output. Any secret in output = leaked.
128+
- `echo $VAR`, `printenv`, `env`, `set`, `Get-ChildItem Env:` = CRITICAL risk (leaks secrets to container)
129+
- Writing environment variables to files in shared/mounted directories = CRITICAL risk
130+
- Commands whose output or error messages might contain secrets = HIGH risk
131+
- Piping sensitive data anywhere = CRITICAL risk
132+
- Legitimate USE of env vars (like `git push` using tokens internally) = LOW risk
127133
""";
128134

129135
// Suppress CA1822 - this is a DI service, keeping as instance method for consistency

0 commit comments

Comments
 (0)