fix(security): Code Review P0+P1+P2 全修補 — MCP Phase 2b-3 + decision_manager
Some checks failed
CD Pipeline / build-and-deploy (push) Has been cancelled
Some checks failed
CD Pipeline / build-and-deploy (push) Has been cancelled
P0: decision_manager _fetch_metrics_snapshot 參數型別錯誤
- prom._instant_query(str) → prom._instant_query({"query": str})
- 結果解析 r.get("status")=="success" → r.get("result", [])
P1: prometheus_provider — alertname PromQL injection 防範
- 新增 _RE_SAFE_ALERTNAME 白名單正則
P1: decision_manager — kubectl action 危險字元注入防範
- 新增 _ALLOWED_KUBECTL_PATTERN 白名單,非法指令格式直接拒絕
P1: decision_manager — 6 個 asyncio.create_task() GC 風險
- 新增 _background_tasks: set + _fire_and_forget() helper
- 所有 bare create_task 改用 _fire_and_forget
P1: ssh_provider — Group B 寫入工具強制需要 known_hosts
- known_hosts 未設定或檔案不存在時拒絕執行,防 MITM
P2: sentry_provider — query 語意白名單驗證
- 新增 _RE_SAFE_SENTRY_QUERY,拒絕含特殊字元的 query
P2: argocd_provider — verify=False 改為 ARGOCD_VERIFY_TLS 環境變數開關
- 新增 _tls_verify() helper,預設 false(self-signed cert)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -33,6 +33,12 @@ _HTTP_TIMEOUT = 10.0
|
||||
_RE_SAFE_APP_NAME = re.compile(r'^[a-zA-Z0-9][a-zA-Z0-9._-]{0,62}$')
|
||||
|
||||
|
||||
def _tls_verify() -> bool:
|
||||
"""P2 fix 2026-04-11: 讀取 ARGOCD_VERIFY_TLS env,預設 False(因 self-signed cert)。"""
|
||||
import os
|
||||
return os.environ.get("ARGOCD_VERIFY_TLS", "false").lower() in ("true", "1", "yes")
|
||||
|
||||
|
||||
def _validate_app_name(name: str) -> str:
|
||||
if not _RE_SAFE_APP_NAME.match(name):
|
||||
raise ValueError(f"Unsafe app name: {name!r}")
|
||||
@@ -152,7 +158,7 @@ class ArgoCDProvider(MCPToolProvider):
|
||||
if ns_filter:
|
||||
params["appNamespace"] = ns_filter
|
||||
|
||||
async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT, verify=False) as client:
|
||||
async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT, verify=_tls_verify()) as client:
|
||||
resp = await client.get(url, headers=self._headers(), params=params)
|
||||
resp.raise_for_status()
|
||||
data = resp.json()
|
||||
@@ -183,7 +189,7 @@ class ArgoCDProvider(MCPToolProvider):
|
||||
app_name = _validate_app_name(parameters["app_name"])
|
||||
url = f"{self._base_url()}/api/v1/applications/{app_name}"
|
||||
|
||||
async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT, verify=False) as client:
|
||||
async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT, verify=_tls_verify()) as client:
|
||||
resp = await client.get(url, headers=self._headers())
|
||||
resp.raise_for_status()
|
||||
app = resp.json()
|
||||
@@ -229,7 +235,7 @@ class ArgoCDProvider(MCPToolProvider):
|
||||
|
||||
# ArgoCD history endpoint
|
||||
history_url = f"{self._base_url()}/api/v1/applications/{app_name}"
|
||||
async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT, verify=False) as client:
|
||||
async with httpx.AsyncClient(timeout=_HTTP_TIMEOUT, verify=_tls_verify()) as client:
|
||||
resp = await client.get(history_url, headers=self._headers())
|
||||
resp.raise_for_status()
|
||||
app = resp.json()
|
||||
@@ -255,7 +261,7 @@ class ArgoCDProvider(MCPToolProvider):
|
||||
async def health_check(self) -> bool:
|
||||
try:
|
||||
url = f"{self._base_url()}/api/v1/applicationsets"
|
||||
async with httpx.AsyncClient(timeout=5.0, verify=False) as client:
|
||||
async with httpx.AsyncClient(timeout=5.0, verify=_tls_verify()) as client:
|
||||
resp = await client.get(url, headers=self._headers())
|
||||
return resp.status_code < 500
|
||||
except Exception:
|
||||
|
||||
@@ -19,6 +19,7 @@ Prometheus MCP Tool Provider — MCP Phase 2b
|
||||
@see docs/superpowers/specs/2026-04-10-infra-rebuild-sprint-abc-design.md §MCP-2b
|
||||
"""
|
||||
|
||||
import re
|
||||
import uuid
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from typing import Any
|
||||
@@ -26,6 +27,9 @@ from typing import Any
|
||||
import httpx
|
||||
import structlog
|
||||
|
||||
# P1 fix 2026-04-11: alertname 白名單防止 PromQL label injection
|
||||
_RE_SAFE_ALERTNAME = re.compile(r"^[a-zA-Z0-9_-]{1,64}$")
|
||||
|
||||
from src.plugins.mcp.interfaces import MCPTool, MCPToolProvider, MCPToolResult
|
||||
|
||||
logger = structlog.get_logger(__name__)
|
||||
@@ -248,6 +252,9 @@ class PrometheusProvider(MCPToolProvider):
|
||||
|
||||
async def _alert_history(self, params: dict) -> dict:
|
||||
alertname = params["alertname"]
|
||||
# P1 fix 2026-04-11: 白名單驗證防止 PromQL label injection
|
||||
if not _RE_SAFE_ALERTNAME.match(alertname):
|
||||
raise ValueError(f"Unsafe alertname: {alertname!r}")
|
||||
window_hours = int(params.get("window_hours", 24))
|
||||
limit = int(params.get("limit", 20))
|
||||
|
||||
|
||||
@@ -31,6 +31,11 @@ logger = structlog.get_logger(__name__)
|
||||
|
||||
_HTTP_TIMEOUT = 10.0
|
||||
_RE_SAFE_ISSUE_ID = re.compile(r'^\d{1,20}$')
|
||||
# P2 fix 2026-04-11: Sentry query 語意白名單 — 只允許已知安全的 Sentry 搜尋語法
|
||||
# 允許: 純文字識別字、is:unresolved、level:error、project:xxx、assigned:me 等
|
||||
_RE_SAFE_SENTRY_QUERY = re.compile(
|
||||
r'^[\w\s:.\-/]+$' # alphanumeric, space, colon, dot, dash, slash — Sentry filter 所需
|
||||
)
|
||||
|
||||
|
||||
class SentryProvider(MCPToolProvider):
|
||||
@@ -222,7 +227,10 @@ class SentryProvider(MCPToolProvider):
|
||||
return MCPToolResult(success=True, data=result)
|
||||
|
||||
async def _search_issues(self, parameters: dict) -> MCPToolResult:
|
||||
query = str(parameters["query"])[:200] # 限制長度防止注入
|
||||
query = str(parameters["query"])[:200]
|
||||
# P2 fix 2026-04-11: 語意白名單驗證,拒絕含特殊字元的 query
|
||||
if not _RE_SAFE_SENTRY_QUERY.match(query):
|
||||
return MCPToolResult(success=False, error=f"Unsafe sentry query: {query!r}")
|
||||
limit = max(1, min(int(parameters.get("limit", 10)), 25))
|
||||
|
||||
url = f"{self._base_url()}/api/0/projects/{self._org()}/{self._project()}/issues/"
|
||||
|
||||
@@ -360,6 +360,21 @@ class SSHProvider(MCPToolProvider):
|
||||
),
|
||||
)
|
||||
|
||||
# P1 fix 2026-04-11: 群組 B 寫入工具必須有 known_hosts,否則拒絕執行
|
||||
# 防止 MITM — 讀取工具(Group A)允許 known_hosts=None 方便診斷;寫入操作不行
|
||||
import os as _os
|
||||
if tool_name in GROUP_B_TOOLS:
|
||||
_kh = _os.environ.get("SSH_MCP_KNOWN_HOSTS_FILE")
|
||||
if not _kh or not _os.path.exists(_kh):
|
||||
return MCPToolResult(
|
||||
success=False,
|
||||
execution_id=execution_id,
|
||||
error=(
|
||||
"Group B write tool refused: SSH_MCP_KNOWN_HOSTS_FILE not set or missing. "
|
||||
"Set up known_hosts per docs/runbooks/ssh-mcp-setup.md before write operations."
|
||||
),
|
||||
)
|
||||
|
||||
# 執行
|
||||
try:
|
||||
is_group_b = tool_name in GROUP_B_TOOLS
|
||||
|
||||
Reference in New Issue
Block a user