Skip to content

Commit 3f2420f

Browse files
authored
fix: security fixes (#9)
1 parent e06ed87 commit 3f2420f

File tree

16 files changed

+764
-71
lines changed

16 files changed

+764
-71
lines changed

DEVELOPMENT.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,20 @@ To use Auth0 as your identity provider:
115115

116116
For external validation (e.g., via a proxy), set `strategy = "external"` and configure your proxy to forward validated JWTs in the `X-Validated-Jwt` header.
117117

118+
### Configuring Exposed JWT Claims
119+
120+
As a template, this project allows configuring which JWT claims are exposed to MCP tools via the `context` module. This is crucial for security, as JWT payloads may contain sensitive information (PII) that should not be accessible to tools.
121+
122+
- **Default**: `jwt_exposed_claims = "all"` - Exposes all claims in the JWT payload.
123+
- **Secure Option**: `jwt_exposed_claims = ["user_id", "roles", "permissions"]` - Only exposes specific claims.
124+
125+
**Important**: Review your JWT structure and set `jwt_exposed_claims` to only the claims your tools need. Avoid exposing sensitive data like emails, personal info, or internal IDs unless necessary. Update this in `config.toml` and test that tools receive only expected data.
126+
127+
Example in `config.toml`:
128+
```toml
129+
jwt_exposed_claims = ["user_id", "roles"]
130+
```
131+
118132
## Configuration Placeholders
119133

120134
Before using this template, you must replace all placeholders with your actual values:

README-es.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,52 @@ Ver `config.toml` para ejemplo de configuración.
158158

159159
**Nota de seguridad**: Por defecto, el servidor se ejecuta en `127.0.0.1` para evitar exposiciones no deseadas. Cambia a `0.0.0.0` solo si es necesario y con las medidas de seguridad apropiadas.
160160

161+
## Consideraciones de Seguridad
162+
163+
Esta plantilla implementa varias medidas de seguridad para proteger contra vulnerabilidades comunes. Como plantilla, está diseñada para ser configurable para diferentes escenarios de despliegue.
164+
165+
### Exposición de Claims JWT
166+
167+
Para minimizar la exposición de datos, configura qué claims JWT son accesibles en el contexto:
168+
169+
```toml
170+
[context]
171+
jwt_exposed_claims = ["user_id", "roles"] # Solo exponer claims específicos
172+
# o
173+
jwt_exposed_claims = "all" # Exponer todos los claims (no recomendado para producción)
174+
```
175+
176+
### Logging de Acceso
177+
178+
Los headers sensibles se redactan automáticamente en los logs:
179+
180+
```toml
181+
[logging]
182+
redacted_headers = ["Authorization", "X-API-Key", "Cookie"]
183+
max_body_size = 1024 # Limitar el tamaño del body logueado
184+
```
185+
186+
### Rate Limiting
187+
188+
Se implementa rate limiting básico para validación JWT para prevenir ataques de fuerza bruta.
189+
190+
### Validación de URIs
191+
192+
Las URIs de OAuth y JWKS se validan contra dominios en whitelist para prevenir ataques SSRF.
193+
194+
### Dependencias Seguras
195+
196+
Las dependencias se actualizan regularmente para abordar vulnerabilidades conocidas. Ejecuta `uv lock --upgrade` para actualizar a las versiones más recientes seguras.
197+
198+
### Lista de Verificación para Producción
199+
200+
- Usar estrategia JWT "external" con un proxy apropiado (Istio, Envoy)
201+
- Configurar claims expuestos mínimos
202+
- Habilitar logging de acceso con redacción
203+
- Validar todas las URIs contra dominios confiables
204+
- Mantener dependencias actualizadas
205+
- Ejecutar tests de seguridad regularmente
206+
161207
## Documentación
162208

163209
- [Documentación Completa](docs/index.md) - Guía completa incluyendo desarrollo, configuración y contribución.

README.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,52 @@ See `config.toml` for configuration example.
160160

161161
**Security Note**: By default, the server runs on `127.0.0.1` to avoid unwanted exposures. Change to `0.0.0.0` only if necessary and with appropriate security measures.
162162

163+
## Security Considerations
164+
165+
This template implements several security measures to protect against common vulnerabilities. As a template, it's designed to be configurable for different deployment scenarios.
166+
167+
### JWT Claims Exposure
168+
169+
To minimize data exposure, configure which JWT claims are accessible in the context:
170+
171+
```toml
172+
[context]
173+
jwt_exposed_claims = ["user_id", "roles"] # Only expose specific claims
174+
# or
175+
jwt_exposed_claims = "all" # Expose all claims (not recommended for production)
176+
```
177+
178+
### Access Logging
179+
180+
Sensitive headers are automatically redacted in logs:
181+
182+
```toml
183+
[logging]
184+
redacted_headers = ["Authorization", "X-API-Key", "Cookie"]
185+
max_body_size = 1024 # Limit logged body size
186+
```
187+
188+
### Rate Limiting
189+
190+
Basic rate limiting is implemented for JWT validation to prevent brute force attacks.
191+
192+
### URI Validation
193+
194+
OAuth and JWKS URIs are validated against whitelisted domains to prevent SSRF attacks.
195+
196+
### Secure Dependencies
197+
198+
Dependencies are regularly updated to address known vulnerabilities. Run `uv lock --upgrade` to update to latest secure versions.
199+
200+
### Production Checklist
201+
202+
- Use "external" JWT strategy with a proper proxy (Istio, Envoy)
203+
- Configure minimal exposed claims
204+
- Enable access logging with redaction
205+
- Validate all URIs against trusted domains
206+
- Keep dependencies updated
207+
- Run security tests regularly
208+
163209
## Documentation
164210

165211
- [Full Documentation](docs/index.md) - Complete guide including development, configuration, and contributing.

config.toml

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,15 @@ enabled = true
2121
strategy = "local"
2222
forwarded_header = "X-Validated-Jwt"
2323

24+
# JWT Claims Exposure Configuration
25+
# Set to "all" to expose all claims, or list specific claims to expose only those
26+
# IMPORTANT: Review and limit exposed claims to prevent PII leakage
27+
jwt_exposed_claims = "all"
28+
2429
[middleware.jwt.validation.local]
2530
jwks_uri = "https://your-keycloak.example.com/realms/your-realm/protocol/openid-connect/certs"
2631
cache_interval = 10
32+
whitelist_domains = ["your-keycloak.example.com"]
2733

2834
[[middleware.jwt.validation.local.allow_conditions]]
2935
expression = 'has(payload.email) && payload.email.endswith("@yourdomain.com")'
@@ -39,3 +45,7 @@ resource = "mcp-resource"
3945
auth_servers = ["http://localhost:8080/auth/realms/master"]
4046
jwks_uri = "http://localhost:8080/auth/realms/master/protocol/openid-connect/certs"
4147
scopes_supported = ["openid", "profile", "email"]
48+
49+
# OAuth Whitelist Domains
50+
# List of allowed domains for OAuth URIs to prevent SSRF attacks
51+
oauth_whitelist_domains = ["localhost", "yourdomain.com"]

src/mcp_app/config.py

Lines changed: 50 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,9 @@
99

1010
from __future__ import annotations
1111

12+
import logging
1213
import os
14+
import re
1315
from datetime import timedelta # noqa: TC003
1416
from pathlib import Path
1517

@@ -121,11 +123,43 @@ class Configuration(BaseModel):
121123
middleware: MiddlewareConfig | None = None
122124
oauth_authorization_server: OAuthAuthorizationServer | None = None
123125
oauth_protected_resource: OAuthProtectedResourceConfig | None = None
126+
jwt_exposed_claims: str | list[str] = "all"
127+
oauth_whitelist_domains: list[str] = []
124128

125129

126130
# --- Configuration Loading Function ---
127131

128132

133+
def safe_expandvars(content: str, allowed_vars: set[str] | None = None) -> str:
134+
"""
135+
Safely expand environment variables in content.
136+
137+
Only expands variables that are in the allowed_vars set to prevent accidental
138+
exposure of sensitive environment variables.
139+
140+
Args:
141+
content: The string content to expand variables in.
142+
allowed_vars: Set of allowed environment variable names. If None, allows all.
143+
144+
Returns:
145+
The content with allowed variables expanded.
146+
147+
"""
148+
149+
def replacer(match: re.Match[str]) -> str:
150+
var_name = match.group(1) or match.group(2)
151+
if allowed_vars is not None and var_name not in allowed_vars:
152+
# Log warning but don't expand
153+
logger = logging.getLogger(__name__)
154+
logger.warning("Blocked expansion of disallowed env var: %s", var_name)
155+
return match.group(0) # Return original ${VAR} or $VAR unchanged
156+
return os.environ.get(var_name, match.group(0))
157+
158+
# Match ${VAR} or $VAR patterns
159+
pattern = re.compile(r"\$\{([^}]+)\}|\$([A-Za-z_][A-Za-z0-9_]*)")
160+
return pattern.sub(replacer, content)
161+
162+
129163
def load_config_from_file(filepath: str | Path) -> Configuration:
130164
"""
131165
Read a TOML configuration file.
@@ -145,7 +179,22 @@ def load_config_from_file(filepath: str | Path) -> Configuration:
145179
raise FileNotFoundError(msg)
146180

147181
raw_content = config_path.read_text(encoding="utf-8")
148-
expanded_content = os.path.expandvars(raw_content)
182+
# Only allow expansion of common, non-sensitive vars
183+
allowed_vars = {
184+
"HOME",
185+
"USER",
186+
"PATH",
187+
"PWD",
188+
"SHELL",
189+
"LANG",
190+
"LC_ALL",
191+
"TMPDIR",
192+
"TEMP",
193+
"TMP",
194+
"LOGNAME",
195+
"USERNAME",
196+
}
197+
expanded_content = safe_expandvars(raw_content, allowed_vars)
149198
config_data = tomllib.loads(expanded_content)
150199

151200
return Configuration.model_validate(config_data)

src/mcp_app/context.py

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,22 +8,64 @@
88
from contextvars import ContextVar
99
from typing import Any
1010

11+
12+
class JWTContextConfig:
13+
"""Configuration for JWT context exposure."""
14+
15+
def __init__(self) -> None:
16+
"""Initialize the JWT context configuration."""
17+
self.exposed_claims: str | list[str] = "all"
18+
19+
20+
# Singleton instance
21+
jwt_context_config = JWTContextConfig()
22+
1123
# Context variables for JWT data
1224
jwt_token: ContextVar[str | None] = ContextVar("jwt_token", default=None)
1325
jwt_payload: ContextVar[dict[str, Any] | None] = ContextVar("jwt_payload", default=None)
1426

1527

28+
def set_exposed_claims(claims: str | list[str]) -> None:
29+
"""
30+
Set the configuration for which JWT claims to expose.
31+
32+
Args:
33+
claims: "all" to expose all claims, or a list of claim names to expose only those.
34+
35+
"""
36+
jwt_context_config.exposed_claims = claims
37+
38+
39+
def filter_payload(payload: dict[str, Any]) -> dict[str, Any]:
40+
"""
41+
Filter the JWT payload based on exposed_claims configuration.
42+
43+
Args:
44+
payload: The full JWT payload.
45+
46+
Returns:
47+
Filtered payload dictionary.
48+
49+
"""
50+
if jwt_context_config.exposed_claims == "all":
51+
return payload
52+
if isinstance(jwt_context_config.exposed_claims, list):
53+
return {k: v for k, v in payload.items() if k in jwt_context_config.exposed_claims}
54+
# Fallback to all if misconfigured
55+
return payload
56+
57+
1658
def set_jwt_context(token: str, payload: dict[str, Any]) -> None:
1759
"""
18-
Set the JWT token and its decoded payload.
60+
Set the JWT token and its decoded payload, filtering based on configuration.
1961
2062
Args:
2163
token: The validated JWT token string.
2264
payload: The decoded JWT payload.
2365
2466
"""
2567
jwt_token.set(token)
26-
jwt_payload.set(payload)
68+
jwt_payload.set(filter_payload(payload))
2769

2870

2971
def get_jwt_payload() -> dict[str, Any] | None:

src/mcp_app/handlers/handlers.py

Lines changed: 51 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,16 @@
55
corresponding to the Go handlers.
66
"""
77

8+
import logging
9+
from urllib.parse import urlparse
10+
811
import httpx
912
from fastapi import HTTPException
1013

1114
from mcp_app.config import Configuration
1215

16+
logger = logging.getLogger(__name__)
17+
1318

1419
class HandlersManager:
1520
"""Manager for OAuth handlers."""
@@ -18,6 +23,32 @@ def __init__(self, config: Configuration) -> None:
1823
"""Initialize the handlers manager."""
1924
self.config = config
2025

26+
def _is_uri_allowed(self, uri: str) -> bool:
27+
"""Check if URI domain is in OAuth whitelist."""
28+
if not self.config.oauth_whitelist_domains:
29+
return True # Allow all if no whitelist
30+
parsed = urlparse(uri)
31+
domain = parsed.netloc
32+
return any(domain.endswith(allowed) for allowed in self.config.oauth_whitelist_domains)
33+
34+
def _sanitize_openid_config(self, data: dict) -> dict:
35+
"""Sanitize OpenID configuration response."""
36+
# Remove potentially sensitive fields like private keys, secrets, etc.
37+
sensitive_fields = {
38+
"private_key_jwt",
39+
"client_secret",
40+
"registration_access_token",
41+
"introspection_endpoint_auth_signing_alg_values_supported",
42+
# Add more as needed
43+
}
44+
sanitized = {}
45+
for key, value in data.items():
46+
if key not in sensitive_fields:
47+
sanitized[key] = value
48+
else:
49+
logger.warning("Removed sensitive field from OpenID config: %s", key)
50+
return sanitized
51+
2152
async def handle_oauth_authorization_server(self) -> dict:
2253
"""
2354
Handle requests for /.well-known/oauth-authorization-server endpoint.
@@ -30,19 +61,22 @@ async def handle_oauth_authorization_server(self) -> dict:
3061
):
3162
raise HTTPException(status_code=404, detail="OAuth authorization server not enabled")
3263

33-
remote_url = (
34-
f"{self.config.oauth_authorization_server.issuer_uri}/.well-known/openid-configuration"
35-
)
64+
issuer_uri = self.config.oauth_authorization_server.issuer_uri
65+
if not self._is_uri_allowed(issuer_uri):
66+
raise HTTPException(status_code=403, detail="Issuer URI not in allowed domains")
3667

37-
async with httpx.AsyncClient() as client:
68+
remote_url = f"{issuer_uri}/.well-known/openid-configuration"
69+
70+
async with httpx.AsyncClient(timeout=10.0) as client:
3871
try:
3972
response = await client.get(remote_url)
4073
response.raise_for_status()
41-
return response.json()
74+
data = response.json()
75+
# Sanitize response: remove potentially sensitive fields
76+
return self._sanitize_openid_config(data)
4277
except httpx.HTTPError as e:
43-
raise HTTPException(
44-
status_code=500, detail=f"Error fetching OpenID config: {e!s}"
45-
) from e
78+
logger.exception("Error fetching OpenID config from %s", remote_url)
79+
raise HTTPException(status_code=500, detail="Error fetching OpenID config") from e
4680

4781
async def handle_oauth_protected_resources(self) -> dict:
4882
"""
@@ -58,6 +92,15 @@ async def handle_oauth_protected_resources(self) -> dict:
5892

5993
pr = self.config.oauth_protected_resource
6094

95+
# Validate URIs
96+
for auth_server in pr.auth_servers:
97+
if not self._is_uri_allowed(auth_server):
98+
raise HTTPException(
99+
status_code=403, detail=f"Auth server URI not allowed: {auth_server}"
100+
)
101+
if not self._is_uri_allowed(pr.jwks_uri):
102+
raise HTTPException(status_code=403, detail="JWKS URI not allowed")
103+
61104
response = {
62105
"resource": pr.resource,
63106
"authorization_servers": pr.auth_servers,

0 commit comments

Comments
 (0)