[codex] Fix audit findings for defense readiness

.github/workflows/ci.yml

@@ -47,7 +47,12 @@ jobs:
       - name: Install dependencies
         run: poetry install --no-interaction --no-ansi
       - name: Run pytest
-        run: poetry run pytest --ignore=tests/integration
+        run: >
+          poetry run pytest --ignore=tests/integration
+          --cov=clickadvisor
+          --cov-report=term-missing
+          --cov-report=xml
+          --cov-fail-under=80
 
   integration-test:
     runs-on: ubuntu-latest

clickadvisor/cli/main.py

@@ -63,16 +63,38 @@ def mcp_http_server(
     host: Annotated[str, typer.Option(help="Bind host")] = "127.0.0.1",
     port: Annotated[int, typer.Option(help="Bind port")] = 8765,
     path: Annotated[str, typer.Option(help="MCP endpoint path")] = "/mcp",
+    bearer_token: Annotated[
+        str | None,
+        typer.Option(help="Bearer token required by Streamable HTTP clients"),
+    ] = None,
+    rate_limit_per_minute: Annotated[
+        int,
+        typer.Option(help="Per-client HTTP request limit per minute"),
+    ] = 60,
+    allow_detect_version: Annotated[
+        bool | None,
+        typer.Option(
+            "--allow-detect-version/--disable-detect-version",
+            help="Expose detect_ch_version over HTTP. Defaults to local-only.",
+        ),
+    ] = None,
 ) -> None:
     """Запустить Streamable HTTP MCP сервер для remote-compatible demo."""
     from clickadvisor.mcp_server.server import run_http
 
     if host not in {"127.0.0.1", "localhost"}:
         console.print(
-            "[yellow]Внимание: remote MCP endpoint должен быть защищён HTTPS/auth proxy. "
+            "[yellow]Внимание: remote MCP endpoint требует bearer token. "
             "Для локального demo безопаснее использовать 127.0.0.1.[/yellow]"
         )
-    run_http(host=host, port=port, path=path)
+    run_http(
+        host=host,
+        port=port,
+        path=path,
+        bearer_token=bearer_token,
+        rate_limit_per_minute=rate_limit_per_minute,
+        allow_detect_version=allow_detect_version,
+    )
 
 
 @app.command()

clickadvisor/mcp_server/server.py

@@ -2,6 +2,9 @@
 
 import asyncio
 import json
+import os
+import secrets
+import time
 from pathlib import Path
 from typing import Any, cast
 
@@ -19,6 +22,83 @@
 
 server = Server("clickadvisor")
 
+LOCAL_HTTP_HOSTS = {"127.0.0.1", "localhost", "::1"}
+
+
+def _env_bool(name: str, *, default: bool) -> bool:
+    raw = os.environ.get(name)
+    if raw is None:
+        return default
+    return raw.strip().lower() in {"1", "true", "yes", "on"}
+
+
+def _env_int(name: str, *, default: int) -> int:
+    raw = os.environ.get(name)
+    if raw is None:
+        return default
+    try:
+        value = int(raw)
+    except ValueError:
+        return default
+    return value if value > 0 else default
+
+
+async def _send_json_error(send: Any, *, status: int, message: str) -> None:
+    body = json.dumps({"error": message}, ensure_ascii=False).encode("utf-8")
+    await send(
+        {
+            "type": "http.response.start",
+            "status": status,
+            "headers": [
+                (b"content-type", b"application/json; charset=utf-8"),
+                (b"content-length", str(len(body)).encode("ascii")),
+            ],
+        }
+    )
+    await send({"type": "http.response.body", "body": body})
+
+
+class HttpSecurityMiddleware:
+    def __init__(
+        self,
+        app: Any,
+        *,
+        bearer_token: str | None = None,
+        rate_limit_per_minute: int = 60,
+    ) -> None:
+        self.app = app
+        self.bearer_token = bearer_token
+        self.rate_limit_per_minute = rate_limit_per_minute
+        self._hits_by_client: dict[str, list[float]] = {}
+
+    async def __call__(self, scope: dict[str, Any], receive: Any, send: Any) -> None:
+        if scope.get("type") != "http":
+            await self.app(scope, receive, send)
+            return
+
+        if self.bearer_token:
+            headers = {
+                key.decode("latin1").lower(): value.decode("latin1")
+                for key, value in scope.get("headers", [])
+            }
+            expected = f"Bearer {self.bearer_token}"
+            if not secrets.compare_digest(headers.get("authorization", ""), expected):
+                await _send_json_error(send, status=401, message="missing or invalid bearer token")
+                return
+
+        client = scope.get("client")
+        client_host = str(client[0]) if client else "unknown"
+        now = time.monotonic()
+        window_start = now - 60.0
+        hits = [stamp for stamp in self._hits_by_client.get(client_host, []) if stamp >= window_start]
+        if len(hits) >= self.rate_limit_per_minute:
+            await _send_json_error(send, status=429, message="rate limit exceeded")
+            return
+        hits.append(now)
+        self._hits_by_client[client_host] = hits
+
+        await self.app(scope, receive, send)
+
 
 @server.list_tools()  # type: ignore[no-untyped-call, untyped-decorator]
 async def list_tools() -> list[Tool]:
@@ -28,7 +108,7 @@ async def list_tools() -> list[Tool]:
             description=(
                 "Анализирует ClickHouse SQL-запрос на антипаттерны и проблемы производительности. "
                 "Возвращает структурированный отчёт с формально обоснованными рекомендациями. "
-                "Использует библиотеку из 20+ правил из реляционной алгебры и инвариантов ClickHouse. "
+                "Использует библиотеку из 119 ClickHouse-specific правил и детекторов. "
                 "Не отправляет данные во внешние сервисы — работает локально. "
                 "ВАЖНО: если пользователь упомянул версию ClickHouse в разговоре — "
                 "всегда передавай её в ch_version. Если упомянул адрес кластера — "
@@ -92,7 +172,7 @@ async def list_tools() -> list[Tool]:
                 "properties": {
                     "tier": {
                         "type": "string",
-                        "enum": ["1A", "1B", "1C", "detector", "all"],
+                        "enum": ["1A", "1B", "1C", "2", "detector", "env", "all"],
                         "description": "Фильтр по типу правил",
                         "default": "all",
                     }
@@ -276,7 +356,7 @@ async def _list_rules(arguments: dict[str, Any]) -> list[TextContent]:
     rules = _get_applicable_rules(None)
 
     lines = ["# Правила оптимизации ClickAdvisor\n"]
-    tier_order = {"1A": 0, "1B": 1, "1C": 2, "detector": 3, "rag": 4}
+    tier_order = {"1A": 0, "1B": 1, "1C": 2, "2": 3, "detector": 4, "env": 5, "rag": 6}
     sorted_rules = sorted(rules, key=lambda rule: (tier_order.get(rule.tier, 5), rule.rule_id))
 
     current_tier = None
@@ -289,7 +369,9 @@ async def _list_rules(arguments: dict[str, Any]) -> list[TextContent]:
                 "1A": "## Tier 1A — Формально эквивалентные (применяются автоматически)",
                 "1B": "## Tier 1B — Приближённые (требуют opt-in)",
                 "1C": "## Tier 1C — Условные (зависят от схемы или контекста)",
+                "2": "## Tier 2 — Cost / design advisory",
                 "detector": "## Детекторы — Антипаттерны",
+                "env": "## Environment checks — Настройки и окружение",
             }
             lines.append(tier_labels.get(current_tier, f"## {current_tier}"))
         lines.append(
@@ -349,6 +431,7 @@ def build_fastmcp_server(
     host: str = "127.0.0.1",
     port: int = 8765,
     path: str = "/mcp",
+    allow_detect_version: bool = True,
 ) -> FastMCP:
     mcp = FastMCP(
         "clickadvisor",
@@ -394,20 +477,22 @@ async def list_rules_http(tier: str = "all") -> str:
         result = await _list_rules({"tier": tier})
         return result[0].text
 
-    @mcp.tool(name="detect_ch_version")
-    async def detect_ch_version_http(
-        connect_url: str,
-        user: str = "default",
-        password: str = "",
-    ) -> str:
-        result = await _detect_ch_version(
-            {
-                "connect_url": connect_url,
-                "user": user,
-                "password": password,
-            }
-        )
-        return result[0].text
+    if allow_detect_version:
+
+        @mcp.tool(name="detect_ch_version")
+        async def detect_ch_version_http(
+            connect_url: str,
+            user: str = "default",
+            password: str = "",
+        ) -> str:
+            result = await _detect_ch_version(
+                {
+                    "connect_url": connect_url,
+                    "user": user,
+                    "password": password,
+                }
+            )
+            return result[0].text
 
     @mcp.prompt(name="analyze")
     def analyze_prompt(sql: str, ch_version: str = "", mode: str = "diagnose") -> str:
@@ -433,8 +518,64 @@ def explain_prompt(sql: str, ch_version: str = "") -> str:
     return mcp
 
 
-def run_http(*, host: str = "127.0.0.1", port: int = 8765, path: str = "/mcp") -> None:
-    build_fastmcp_server(host=host, port=port, path=path).run(transport="streamable-http")
+def build_streamable_http_app(
+    *,
+    host: str = "127.0.0.1",
+    port: int = 8765,
+    path: str = "/mcp",
+    bearer_token: str | None = None,
+    rate_limit_per_minute: int = 60,
+    allow_detect_version: bool | None = None,
+) -> Any:
+    if allow_detect_version is None:
+        allow_detect_version = _env_bool(
+            "CLICKADVISOR_MCP_ALLOW_DETECT_VERSION",
+            default=host in LOCAL_HTTP_HOSTS,
+        )
+    mcp = build_fastmcp_server(
+        host=host,
+        port=port,
+        path=path,
+        allow_detect_version=allow_detect_version,
+    )
+    app = mcp.streamable_http_app()
+    return HttpSecurityMiddleware(
+        app,
+        bearer_token=bearer_token,
+        rate_limit_per_minute=rate_limit_per_minute,
+    )
+
+
+def run_http(
+    *,
+    host: str = "127.0.0.1",
+    port: int = 8765,
+    path: str = "/mcp",
+    bearer_token: str | None = None,
+    rate_limit_per_minute: int | None = None,
+    allow_detect_version: bool | None = None,
+) -> None:
+    import uvicorn
+
+    token = bearer_token or os.environ.get("CLICKADVISOR_MCP_BEARER_TOKEN")
+    if host not in LOCAL_HTTP_HOSTS and not token:
+        raise RuntimeError(
+            "CLICKADVISOR_MCP_BEARER_TOKEN is required when binding MCP HTTP "
+            "server to a non-local host."
+        )
+    rate_limit = rate_limit_per_minute or _env_int(
+        "CLICKADVISOR_MCP_RATE_LIMIT_PER_MINUTE",
+        default=60,
+    )
+    app = build_streamable_http_app(
+        host=host,
+        port=port,
+        path=path,
+        bearer_token=token,
+        rate_limit_per_minute=rate_limit,
+        allow_detect_version=allow_detect_version,
+    )
+    uvicorn.run(app, host=host, port=port)
 
 
 async def main() -> None:

docs/MCP.md

@@ -38,10 +38,12 @@ See [MCP Deployment Options](mcp-deployment.md).
 ## Connect To Public Remote MCP
 
 Remote MCP clients connect to ClickAdvisor through the hosted Streamable HTTP
-endpoint:
+endpoint. Public/demo HTTP deployments require a bearer token and are intended
+for synthetic SQL examples, not private production queries:
 
 ```text
 https://clickadvisor-mcp-production.up.railway.app/mcp
+Authorization: Bearer <demo-token>
 ```
 
 Claude / Anthropic API URL-based server config:
@@ -79,6 +81,10 @@ Opening the endpoint in a browser can return `Not Acceptable: Client must
 accept text/event-stream`. This is expected for Streamable HTTP MCP; use an MCP
 client or MCP Inspector to test the tool calls.
 
+Public demo deployments disable `detect_ch_version` by default because it
+accepts arbitrary ClickHouse URLs. Use local stdio MCP or a self-hosted trusted
+HTTP deployment when version detection against private clusters is needed.
+
 ## Connect to Claude Desktop
 
 Add this to `~/Library/Application Support/Claude/claude_desktop_config.json`:

docs/evaluation.md

@@ -35,6 +35,7 @@ multi-label targets from `expected_rules_to_fire`.
 Current command:
 
 ```bash
+poetry install --with ml
 poetry run python scripts/eval/ablation_classifiers.py --run-id classifier_ablation_current
 ```
 
@@ -49,6 +50,7 @@ Current results:
 Artifacts:
 
 - `eval/results/classifier_ablation_current/metrics.json`
+- `eval/results/classifier_ablation_current/metadata.json` with split and metrics SHA-256
 - `eval/results/classifier_ablation_current/metrics.csv`
 - `docs/experiments/classifier_ablation.md`
 
@@ -131,6 +133,13 @@ remain deterministic rule-engine output.
 
 Current baseline ladder:
 
+ML dependencies for this experiment live in the optional Poetry group:
+
+```bash
+poetry install --with ml
+poetry run python scripts/lab/run_risk_baseline_ladder.py
+```
+
 | Model | CV macro-F1 | Test macro-F1 | Holdout macro-F1 |
 |---|---:|---:|---:|
 | dummy_most_frequent | 0.275 +/- 0.000 | 0.276 | 0.278 |
@@ -140,6 +149,34 @@ Current baseline ladder:
 | random_forest_all_features | 0.938 +/- 0.006 | 0.938 | 0.949 |
 | catboost_tabular | 0.873 +/- 0.008 | 0.871 | 0.871 |
 
+Leakage-aware control run:
+
+```bash
+poetry install --with ml
+poetry run python scripts/lab/run_risk_baseline_ladder.py \
+  --run-id risk_baseline_ladder_leakage_aware \
+  --feature-policy leakage_aware \
+  --models dummy_stratified tfidf_logistic_regression random_forest_all_features
+```
+
+`--feature-policy leakage_aware` removes explicit `rule_*` features and
+rule-shaped parser flags such as `base_has_count_distinct`,
+`base_has_final_modifier`, and `has_final`. It keeps neutral query shape/count
+features plus SQL text. This does not make the weak labels independent from the
+rule engine, but it is a stricter leakage-control baseline.
+
+| Model | CV macro-F1 | Test macro-F1 | Holdout macro-F1 |
+|---|---:|---:|---:|
+| dummy_stratified | 0.328 +/- 0.009 | 0.342 | 0.335 |
+| tfidf_logistic_regression | 0.864 +/- 0.011 | 0.869 | 0.882 |
+| random_forest_all_features | 0.879 +/- 0.006 | 0.890 | 0.900 |
+
+The Random Forest drop from `0.949` to `0.900` confirms that part of the
+headline score came from rule-derived features. The remaining lift over the
+stratified dummy baseline comes mostly from SQL text and neutral query-shape
+signals, so it should be presented as triage research rather than an
+independent production classifier.
+
 Important holdout slices for Random Forest:
 
 | Slice | Records | Macro-F1 | High recall |
@@ -182,6 +219,8 @@ Artifacts:
 - `data/ml/expert_dataset/eda/ds_report.md`
 - `docs/experiments/risk_labeling_ds_summary.md`
 - `eval/results/risk_baseline_ladder_current/metrics.json`
+- `eval/results/risk_baseline_ladder_current/metadata.json` with dataset, split, and metrics SHA-256
+- `eval/results/risk_baseline_ladder_leakage_aware/metrics.json`
 - `eval/results/risk_learning_curve_current/summary.md`
 - `data/ml/expert_dataset/eda/risk_error_analysis/error_analysis.md`