Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
510a888
feat: Add simple volume mount validation for Linux
williajm Nov 15, 2025
6a80f9d
feat: Add simple volume mount validation for Linux
williajm Nov 15, 2025
d51043f
chore: Remove accidental Gemini workflows
williajm Nov 15, 2025
983209e
chore: Remove security review files from git, keep locally
williajm Nov 15, 2025
1be97b0
chore: Remove validation proposal from git, keep locally
williajm Nov 15, 2025
c649bbe
test: Add comprehensive unit tests for volume mount validation
williajm Nov 15, 2025
cbb06b0
feat: Enhance credential directory protection with substring matching
williajm Nov 15, 2025
34d1a08
docs: Fix misleading claims about OAuth in startup scripts
williajm Nov 15, 2025
11f8871
security: Fix command injection via environment variables (H1)
williajm Nov 15, 2025
8dd840a
security: Redact environment variable values in prompts (H2)
williajm Nov 15, 2025
a1ff724
security: Prevent rate limiter memory exhaustion DoS (H5)
williajm Nov 15, 2025
2fcfaf3
test: Add list-based command validation tests
williajm Nov 15, 2025
499a96f
security: Set restrictive permissions on audit log files (L2)
williajm Nov 15, 2025
2b634c7
chore: Release v1.1.1
williajm Nov 15, 2025
768d222
refactor: Reduce cognitive complexity in generate_compose prompt
williajm Nov 15, 2025
0e234eb
security: Block path traversal in volume mount validation (P1)
williajm Nov 15, 2025
aaaac64
fix: Cleanup idle clients in rate limiter to prevent permanent DoS
williajm Nov 15, 2025
2cef508
fix: Rate limiter race condition causing CI test failures
williajm Nov 15, 2025
28f5ae4
docs: Add rate limiter race condition fix to CHANGELOG
williajm Nov 15, 2025
3c80beb
fix: P1 - Rate limiter memory exhaustion from unique client IDs
williajm Nov 15, 2025
ec33f3a
fix: P1 - Implement LRU eviction for idle clients in rate limiter
williajm Nov 15, 2025
a64f0d0
docs: Clean up CHANGELOG - remove internal debugging details
williajm Nov 15, 2025
8e19b68
fix: CRITICAL - Apply environment variable validation in CreateContai…
williajm Nov 15, 2025
ceae894
docs: Clean up CHANGELOG - remove internal implementation details
williajm Nov 15, 2025
91ac4d4
fix: Allow ampersands and pipes in environment variables
williajm Nov 15, 2025
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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,8 @@ mcp_audit.*.log.zip
development-plan-*.md
plan*.md
codex_*.md
security_review_*.md
*_validation_proposal.md

# Manual testing guide (git-ignored for local notes)
MANUAL_SSH_TESTING.md
Expand Down
68 changes: 68 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,74 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0

## [Unreleased]

## [1.1.1] - 2025-11-15

### Added
- **Simple volume mount validation**: Prevent accidental mounting of sensitive Linux paths
- **Named volume detection**: Docker-managed volumes always allowed (they're safe)
- **System path blocklist**: Block sensitive system paths (`/etc`, `/root`, `/var/run/docker.sock`)
- **Credential directory protection**: Substring matching blocks credential dirs anywhere in path
- Always blocks: `.ssh`, `.aws`, `.kube`, `.docker` (even under `/home/user/`)
- Prevents accidental exposure of SSH keys, cloud credentials, Kubernetes configs
- **Optional allowlist**: Restrict to specific paths if needed
- **YOLO mode**: `SAFETY_YOLO_MODE=true` bypasses all checks (for advanced users)
- **Linux-focused**: Simple protection for common mistakes, not a security fortress
- Configuration: `SAFETY_VOLUME_MOUNT_BLOCKLIST`, `SAFETY_VOLUME_MOUNT_ALLOWLIST`, `SAFETY_YOLO_MODE`
- **Rate limiter max clients limit**: Prevent memory exhaustion DoS attacks
- New config: `SECURITY_RATE_LIMIT_MAX_CLIENTS` (default: 10, max: 100)
- Rejects new clients when limit reached with clear error message
- Existing clients unaffected at limit
- **Audit log file permissions**: Restrictive permissions on audit logs
- Directory permissions: 0o700 (owner-only access)
- File permissions: 0o600 (owner-only read/write)
- Automatic permission fixing for existing directories

### Security
- **CRITICAL: Command injection via environment variables (H1)**: Prevent command injection in `docker_exec_command`
- Validates environment variables before passing to Docker
- Blocks dangerous characters: `$(`, `` ` ``, `;`, `&`, `|`, `\n`, `\r`
- Prevents exploits like `{"MALICIOUS": "$(cat /etc/passwd)"}`
- **HIGH: Secret leakage in prompts (H2)**: Redact environment variable values in MCP prompts
- `generate_compose` prompt now redacts all env var values
- Shows keys but not values: `DATABASE_URL=<REDACTED>`
- Prevents credential leakage to remote LLM APIs (Claude, OpenAI, etc.)
- Protection is always enabled, cannot be disabled
- Documented in SECURITY.md
- **HIGH: Rate limiter memory exhaustion DoS (H5)**: Prevent unbounded client tracking
- Added `max_clients` limit to rate limiter (default: 10, max: 100)
- Prevents attackers from exhausting memory with many fake client IDs
- Clear error message when limit reached
- **LOW: Audit log file permissions (L2)**: Set restrictive permissions on audit logs
- Directory: 0o700 (was 0o755 - world-readable)
- File: 0o600 (was 0o644 - world-readable)
- Prevents information disclosure on multi-user systems

### Fixed
- **Environment variable validation**: Command injection protection with practical limits
- Validates environment variables for dangerous characters (command substitution, separators)
- Allows ampersands and pipes (common in connection strings like `postgres://...?ssl=true&pool=10`)
- Blocks only truly dangerous patterns: `$(`, backticks, semicolons, newlines
- **Documentation accuracy**: Fixed misleading OAuth claims in startup scripts
- `start-mcp-docker-httpstream.sh` and `start-mcp-docker-sse.sh` documentation
- Clarified that OAuth is disabled by default (set `SECURITY_OAUTH_ENABLED=false`)
- Accurately describe enabled features: TLS, rate limiting, audit logging
- **Rate limiter race condition**: Fixed KeyError in concurrent operations
- Race condition in cleanup logic where concurrent releases deleted semaphore entries
- Fixes CI integration test failures in concurrent operation tests
- **Rate limiter memory exhaustion**: Fixed memory leak from unique client IDs
- Implements LRU eviction of idle clients when at max_clients capacity
- Prevents unbounded memory growth while allowing normal multi-user operation
- Only rejects new clients when all tracked clients have active requests

### Tests
- **20 new unit tests** for volume mount validation (all passing in 0.12s)
- **7 new unit tests** for rate limiter max clients and idle client eviction (all passing)
- **8 new unit tests** for environment variable command injection protection
- **6 new unit tests** for list-based command validation coverage
- **3 new unit tests** for audit log file permissions
- **1 new unit test** for prompt secret redaction
- Total: **45 new tests**, all fast unit tests

## [1.1.0] - 2025-11-14

### Added
Expand Down
68 changes: 66 additions & 2 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ The MCP Docker server implements multiple layers of security:
7. **Error Sanitization** - Prevent information disclosure
8. **Safety Controls** - Three-tier operation classification
9. **HTTP Stream Transport Security** - Session management, DNS rebinding protection, CORS security
10. **Secret Redaction** - Environment variable values redacted in prompts to prevent credential leakage to LLM APIs

## Quick Start

Expand All @@ -40,21 +41,35 @@ Claude Desktop uses stdio transport (local process). The server relies on OS-lev
For production deployment using HTTP Stream Transport with security features:

```bash
# Start server with TLS, OAuth, and security features
# Start server with TLS, rate limiting, and audit logging
# OAuth is DISABLED by default - edit script to enable
./start-mcp-docker-httpstream.sh
```

**What's enabled:**
- ✅ TLS/HTTPS (requires certificates in `~/.mcp-docker/certs/`)
- ✅ Rate limiting (60 requests/minute)
- ✅ Audit logging
- ❌ OAuth/OIDC (disabled by default - see script comments to enable)

See the HTTP Stream Transport Security, OAuth/OIDC Authentication, and TLS/HTTPS sections below for configuration details.

### For Network Deployment (SSE Transport)

For production deployment using SSE transport with security features:

```bash
# Start server with TLS, OAuth, and security features
# Start server with TLS, rate limiting, and audit logging
# OAuth is DISABLED by default - edit script to enable
./start-mcp-docker-sse.sh
```

**What's enabled:**
- ✅ TLS/HTTPS (requires certificates in `~/.mcp-docker/certs/`)
- ✅ Rate limiting (60 requests/minute)
- ✅ Audit logging
- ❌ OAuth/OIDC (disabled by default - see script comments to enable)

See the OAuth/OIDC Authentication and TLS/HTTPS sections below for configuration details.

## OAuth/OIDC Authentication
Expand Down Expand Up @@ -398,6 +413,55 @@ SAFETY_ALLOW_DESTRUCTIVE_OPERATIONS=true
SAFETY_ALLOW_PRIVILEGED_CONTAINERS=true
```

## Secret Redaction in Prompts

**SECURITY**: Environment variable values are automatically redacted in MCP prompts to prevent credential leakage to remote LLM APIs.

### The Risk

When using the `generate_compose` prompt on containers with secrets in environment variables:

```yaml
# Container with secrets
environment:
DATABASE_URL: postgresql://admin:SuperSecret123@db:5432/app
API_KEY: example_api_key_value_here
JWT_SECRET: my-secret-signing-key
```

**Without redaction**: These values would be sent to remote LLM APIs (Claude, OpenAI, etc.) and potentially:
- Logged in provider systems
- Used for model training (depending on provider policies)
- Exposed in API request logs
- Leaked to unauthorized parties

### The Protection

The `generate_compose` prompt automatically **redacts environment variable values**:

```
- Environment Variables: 3 variables (values redacted for security)
- DATABASE_URL=<REDACTED>
- API_KEY=<REDACTED>
- JWT_SECRET=<REDACTED>
```

**What this protects:**
- Database passwords and connection strings
- API keys and tokens
- OAuth secrets
- Encryption keys
- AWS/cloud credentials
- Any sensitive data in environment variables

**How it works:**
- Only environment variable **keys** are shown to the LLM
- All **values** are replaced with `<REDACTED>`
- The LLM can still generate accurate compose files knowing which env vars exist
- No secrets are sent to remote APIs

This protection is **always enabled** and cannot be disabled. If you need to inspect actual environment variable values, use `docker inspect` directly.

## HTTP Stream Transport Security

The HTTP Stream Transport is the modern MCP transport protocol with enhanced security features for network deployments.
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[project]
name = "mcp-docker"
version = "1.1.1.dev0"
version = "1.1.1"
description = "Model Context Protocol server for Docker management with AI assistants"
readme = "README.md"
requires-python = ">=3.11"
Expand Down
49 changes: 45 additions & 4 deletions src/mcp_docker/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -246,19 +246,54 @@ class SafetyConfig(BaseSettings):
),
)

@field_validator("allowed_tools", "denied_tools", mode="before")
# Volume mount validation (simple Linux-focused protection)
yolo_mode: bool = Field(
default=False,
description=(
"Bypass ALL safety checks (user takes full responsibility). "
"Enable for advanced use cases where you need full control."
),
)
volume_mount_blocklist: list[str] = Field(
default_factory=lambda: [
"/etc", # System configuration
"/root", # Root user home
"/var/run/docker.sock", # Docker socket (container escape)
],
description=(
"Blocked volume mount paths (prefix matching, Linux-focused). "
"Note: Credential directories (.ssh, .aws, .kube, .docker) are "
"always blocked via substring matching regardless of this list. "
"Can be set via SAFETY_VOLUME_MOUNT_BLOCKLIST as comma-separated string."
),
)
volume_mount_allowlist: list[str] = Field(
default_factory=list,
description=(
"Allowed volume mount paths (empty = allow all except blocked). "
"Can be set via SAFETY_VOLUME_MOUNT_ALLOWLIST as comma-separated string."
),
)

@field_validator(
"allowed_tools",
"denied_tools",
"volume_mount_blocklist",
"volume_mount_allowlist",
mode="before",
)
@classmethod
def parse_tool_list(cls, value: str | list[str] | None) -> list[str]:
"""Parse tool list from comma-separated string or list.
"""Parse list from comma-separated string or list.

Handles environment variable input as comma-separated strings
and normalizes them to lists.

Args:
value: Tool list as string (comma-separated), list, or None
value: List as string (comma-separated), list, or None

Returns:
Normalized list of tool names (empty list if None/empty)
Normalized list of strings (empty list if None/empty)
"""
if value is None or value == "":
return []
Expand Down Expand Up @@ -298,6 +333,12 @@ class SecurityConfig(BaseSettings):
gt=0,
le=50,
)
rate_limit_max_clients: int = Field(
default=10,
description="Maximum number of unique clients to track (prevents memory exhaustion DoS)",
gt=0,
le=100,
)

# Audit Logging
audit_log_enabled: bool = Field(
Expand Down
77 changes: 48 additions & 29 deletions src/mcp_docker/prompts/templates.py
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,10 @@ class GenerateComposePrompt(BasePromptHelper):
"""Prompt for generating docker-compose.yml files."""

NAME = "generate_compose"
DESCRIPTION = "Generate a docker-compose.yml file from container configuration"
DESCRIPTION = (
"Generate a docker-compose.yml file from container configuration. "
"⚠️ Environment variable values are redacted to prevent secret leakage to LLM APIs."
)

def get_metadata(self) -> PromptMetadata:
"""Get prompt metadata.
Expand Down Expand Up @@ -429,43 +432,38 @@ def _get_container_compose_data_blocking(self, container_id: str) -> dict[str, A
"attrs": container_attrs,
}

async def generate(self, options: GenerateComposeOptions) -> PromptResult:
"""Generate docker-compose.yml from container or description.
def _build_container_context(self, data: dict[str, Any]) -> str:
"""Build context string from container data.

Args:
options: Compose generation options with container_id and service_description
data: Container data with name and attrs

Returns:
Prompt result with compose file generation guidance
Formatted context string with container configuration

"""
context = ""

# If container_id provided, extract its configuration
if options.container_id:
try:
# Offload blocking Docker I/O to thread pool
data = await asyncio.to_thread(
self._get_container_compose_data_blocking, options.container_id
)

# Extract configuration
container_attrs = data["attrs"]
config = container_attrs.get("Config", {})
host_config = container_attrs.get("HostConfig", {})
container_attrs = data["attrs"]
config = container_attrs.get("Config", {})
host_config = container_attrs.get("HostConfig", {})

# Extract key configuration elements
image = config.get("Image", "")
env_vars = config.get("Env") or []
ports = host_config.get("PortBindings") or {}
volumes = host_config.get("Binds") or []
restart_policy = host_config.get("RestartPolicy", {}).get("Name", "no")
network_mode = host_config.get("NetworkMode", "bridge")
# Extract key configuration elements
image = config.get("Image", "")
env_vars = config.get("Env") or []
ports = host_config.get("PortBindings") or {}
volumes = host_config.get("Binds") or []
restart_policy = host_config.get("RestartPolicy", {}).get("Name", "no")
network_mode = host_config.get("NetworkMode", "bridge")

# SECURITY: Redact environment variable values to prevent secret leakage
# Only show keys, not values (e.g., DATABASE_URL=<REDACTED>)
env_vars_redacted = [
var.split("=", 1)[0] + "=<REDACTED>" if "=" in var else var for var in env_vars
]

context = f"""Existing Container Configuration for {data["name"]}:
return f"""Existing Container Configuration for {data["name"]}:
- Image: {image}
- Environment Variables: {len(env_vars)} variables
{chr(10).join(f" - {var}" for var in env_vars[: DISPLAY_LIMITS.env_vars])}
- Environment Variables: {len(env_vars)} variables (values redacted for security)
{chr(10).join(f" - {var}" for var in env_vars_redacted[: DISPLAY_LIMITS.env_vars])}
{" - ..." if len(env_vars) > DISPLAY_LIMITS.env_vars else ""}
- Port Mappings: {len(ports)} ports
{chr(10).join(f" - {k}: {v}" for k, v in list(ports.items())[: DISPLAY_LIMITS.ports])}
Expand All @@ -476,6 +474,27 @@ async def generate(self, options: GenerateComposeOptions) -> PromptResult:
- Restart Policy: {restart_policy}
- Network Mode: {network_mode}
"""

async def generate(self, options: GenerateComposeOptions) -> PromptResult:
"""Generate docker-compose.yml from container or description.

Args:
options: Compose generation options with container_id and service_description

Returns:
Prompt result with compose file generation guidance

"""
context = ""

# If container_id provided, extract its configuration
if options.container_id:
try:
# Offload blocking Docker I/O to thread pool
data = await asyncio.to_thread(
self._get_container_compose_data_blocking, options.container_id
)
context = self._build_container_context(data)
except Exception as e:
logger.error(f"Failed to get container info: {e}")
context = f"Note: Could not retrieve container {options.container_id}: {e}\n"
Expand Down
13 changes: 11 additions & 2 deletions src/mcp_docker/security/audit.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,12 @@ def __init__(self, audit_log_file: Path, enabled: bool = True) -> None:
self.handler_id = None

if self.enabled:
# Ensure parent directory exists
self.audit_log_file.parent.mkdir(parents=True, exist_ok=True)
# Ensure parent directory exists with restrictive permissions
# SECURITY: 0o700 = owner-only access (no group/world read)
self.audit_log_file.parent.mkdir(parents=True, exist_ok=True, mode=0o700)

# Set permissions on existing directory (if it already existed)
self.audit_log_file.parent.chmod(0o700)

# Add dedicated audit log handler with loguru
# SECURITY: Uses loguru's battle-tested file rotation and serialization
Expand All @@ -63,6 +67,11 @@ def __init__(self, audit_log_file: Path, enabled: bool = True) -> None:
diagnose=False, # Don't expose internals
)

# Set restrictive permissions on audit log file
# SECURITY: 0o600 = owner-only read/write (no group/world access)
if self.audit_log_file.exists():
self.audit_log_file.chmod(0o600)

loguru_logger.info(f"Audit logging enabled: {self.audit_log_file}")
else:
loguru_logger.warning("Audit logging DISABLED")
Expand Down
Loading
Loading