From beb30b4bfa78a28d0d7aa022d83dc8ffcc0f2495 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=98=A4=EC=A4=80=EC=84=9D=28Junseok=20Oh=29DevOps?= <52226147+Atom-oh@users.noreply.github.com> Date: Sun, 18 Oct 2026 12:42:17 +0000 Subject: [PATCH 01/27] fix(datasources): bound connector reads and verify real query access Datasource Lambdas cap success (4 MiB) and error (64 KiB) bodies before parsing and close streams on every exit. Missing explicit instance IDs fail closed. Connection tests run the real bounded query operation, and Jaeger/Dynatrace/Datadog/Loki report partial or unverified results instead of absence. Prometheus/Mimir enforce the saved execution ceiling inside the connector. Optional Jaeger/Dynatrace VPC attachment stays default-off. Co-Authored-By: Claude Opus 5.5 --- agent/lambda/clickhouse_mcp.py | 11 +- agent/lambda/datadog_mcp.py | 65 +++-- agent/lambda/datasource_http.py | 87 ++++++- agent/lambda/dynatrace_mcp.py | 72 ++++-- agent/lambda/jaeger_mcp.py | 53 +++- agent/lambda/loki_mcp.py | 86 ++++++- agent/lambda/mimir_mcp.py | 39 ++- agent/lambda/prometheus_mcp.py | 30 ++- agent/lambda/tempo_mcp.py | 18 +- agent/lambda/test_datadog_mcp.py | 81 +++++++ agent/lambda/test_datasource_http.py | 11 +- agent/lambda/test_dynatrace_mcp.py | 46 ++++ agent/lambda/test_external_connections.py | 229 ++++++++++++++++++ agent/lambda/test_mimir_mcp.py | 4 +- agent/lambda/test_prometheus_mcp.py | 4 +- agent/tests/test_health_tools.py | 35 +-- terraform/foundation/ai.tf | 30 ++- .../foundation/tests/runtime_iam.tftest.hcl | 34 +++ 18 files changed, 813 insertions(+), 122 deletions(-) create mode 100644 agent/lambda/test_datadog_mcp.py create mode 100644 agent/lambda/test_dynatrace_mcp.py create mode 100644 agent/lambda/test_external_connections.py diff --git a/agent/lambda/clickhouse_mcp.py b/agent/lambda/clickhouse_mcp.py index a8295ef90..38d08f56f 100644 --- a/agent/lambda/clickhouse_mcp.py +++ b/agent/lambda/clickhouse_mcp.py @@ -273,8 +273,15 @@ def clickhouse_schema(args): def clickhouse_health(args): - """Connectivity probe for the pre-save Test / status badge: GET /ping.""" - return ok(health(load_datasource(SLUG), "/ping")) + """Verify read-only SQL access using one constant row and a one-second ceiling.""" + try: + result = _run_sql("SELECT 1 AS probe", 1, max_execution_time=1) + payload = json.loads(result["body"]) + verified = result["statusCode"] == 200 and payload.get("collectionStatus") in ("ok", "partial") + return ok({"ok": verified, "verified": ["query"] if verified else [], + **({} if verified else {"error": "ClickHouse query access could not be verified"})}) + except Exception: + return ok({"ok": False, "error": "ClickHouse query access could not be verified"}) _TOOLS = { diff --git a/agent/lambda/datadog_mcp.py b/agent/lambda/datadog_mcp.py index 9ef1041ff..faaa8a189 100644 --- a/agent/lambda/datadog_mcp.py +++ b/agent/lambda/datadog_mcp.py @@ -16,7 +16,7 @@ from cross_account import resolve_tool_name from datasource_http import ( NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, http_json, load_datasource, - set_request_conn, + set_request_conn, request_timeout, finite_number, observed_payload, ) SLUG = "datadog" @@ -45,9 +45,9 @@ def _headers(creds): """DD dual-key headers: dedicated apiKey/appKey fields win; else the generic custom-header pair (headerName/headerValue [+2]) from auth_headers covers users who configured them directly.""" h = dict(auth_headers(creds)) - if creds.get("apiKey"): + if creds.get("authType") in (None, "custom_header") and creds.get("apiKey"): h["DD-API-KEY"] = str(creds["apiKey"]) - if creds.get("appKey"): + if creds.get("authType") in (None, "custom_header") and creds.get("appKey"): h["DD-APPLICATION-KEY"] = str(creds["appKey"]) return h @@ -60,11 +60,11 @@ def _ds(): def _get(creds, path, params=None): url = creds["endpoint"].rstrip("/") + path + ("?" + urlencode(params, doseq=True) if params else "") - status, data = http_json("GET", url, headers=_headers(creds)) + status, data = http_json("GET", url, headers=_headers(creds), timeout=request_timeout(creds)) if status >= 400: errs = data.get("errors") if isinstance(data, dict) else None raise _ApiError(f"Datadog HTTP {status}: {str(errs or (data.get('raw') if isinstance(data, dict) else data))[:300]}") - return data + return observed_payload(data, status) def datadog_query(args): @@ -77,18 +77,38 @@ def datadog_query(args): "to": _parse_time_s(args.get("end")), } data = _get(_ds(), "/api/v1/query", params) - series = data.get("series", []) if isinstance(data, dict) else [] + if isinstance(data, dict) and (data.get("status") == "error" or data.get("errors") or data.get("error")): + return ok({"series": [], "truncated": False, "collectionStatus": "error"}) + series = data.get("series") if isinstance(data, dict) else None + if not isinstance(data, dict) or data.get("status") != "ok" or not isinstance(series, list): + return ok({"series": [], "truncated": False, "collectionStatus": "unknown"}) out = [] + truncated = len(series) > MAX_SERIES + invalid = False for s in series[:MAX_SERIES]: - if not isinstance(s, dict): + if not isinstance(s, dict) or not isinstance(s.get("metric"), str) or not isinstance(s.get("pointlist"), list): + invalid = True continue - pts = s.get("pointlist") or [] + pts = s["pointlist"] + truncated = truncated or len(pts) > MAX_POINTS + points = [] + for point in pts[:MAX_POINTS]: + if (not isinstance(point, list) or len(point) != 2 + or not finite_number(point[0], 8640000000000000) + or point[1] is not None and not finite_number(point[1])): + invalid = True + continue + points.append(point) + units = s.get("unit") + unit = units[0].get("name") if isinstance(units, list) and units and isinstance(units[0], dict) else None out.append({ "metric": s.get("metric", ""), "scope": s.get("scope", ""), - "unit": (s.get("unit") or [{}])[0].get("name") if isinstance(s.get("unit"), list) and s.get("unit") else None, - "pointlist": pts[:MAX_POINTS], + "unit": unit, "pointlist": points, }) - return ok({"series": out, "truncated": len(series) > MAX_SERIES, + http_status = getattr(data, "http_status", 200) + state = "unknown" if invalid or http_status not in (200, 206) else "partial" if truncated or http_status == 206 else "ok" if out else "empty" + return ok({"series": out, "truncated": truncated, + "collectionStatus": state, "status": data.get("status") if isinstance(data, dict) else None}) @@ -101,14 +121,27 @@ def datadog_schema(args): def datadog_health(args): - """Key validation probe: GET /api/v1/validate (API key), then a 1-metric list (app key scope).""" + """Validate configured API keys, then metric-query access (key pair or bearer).""" creds = load_datasource(SLUG) assert_host_allowed(creds["endpoint"]) try: - v = _get(creds, "/api/v1/validate") - return ok({"ok": bool(isinstance(v, dict) and v.get("valid"))}) - except _ApiError as e: - return ok({"ok": False, "error": str(e)}) + # The API-key validator does not validate bearer access tokens. Those use + # the supported metric-query operation itself as the authentication proof. + if any(name.lower() == "dd-api-key" for name in _headers(creds)): + v = _get(creds, "/api/v1/validate") + if not isinstance(v, dict) or v.get("valid") is not True: + return ok({"ok": False, "error": "Datadog API key validation failed"}) + end = int(time.time()) + # A one-minute, single-metric query with a probe-only host filter verifies + # metric-query permission without fetching the fleet's real metric series. + probe = _get(creds, "/api/v1/query", { + "query": "avg:system.cpu.user{host:awsops-connection-probe}", + "from": str(end - 60), "to": str(end), + }) + verified = isinstance(probe, dict) and probe.get("status") == "ok" and isinstance(probe.get("series"), list) + return ok({"ok": verified, **({} if verified else {"error": "Datadog metric-query access could not be verified"})}) + except _ApiError: + return ok({"ok": False, "error": "Datadog credentials or metric-query access could not be verified"}) _TOOLS = { diff --git a/agent/lambda/datasource_http.py b/agent/lambda/datasource_http.py index 2c88e91df..f59e7c17b 100644 --- a/agent/lambda/datasource_http.py +++ b/agent/lambda/datasource_http.py @@ -24,9 +24,8 @@ calls assert_host_allowed(endpoint) immediately before http_json(same endpoint) — that invariant is what makes the cache safe; it is reset every invocation by set_request_conn() (called at the top of every lambda_handler) so a warm container never reuses a stale pinned IP across invocations. -(A structurally identical resolve-and-recheck gap exists in agent/agent.py's `_assert_host_allowed` -for the separate egress-integrations transport — untouched here; that's a different call path not -exercised by the pentest's datasource/schema-fetch findings, and needs its own fix.) +The separate custom-MCP transport pins its request-local validated address set through +agent/pinned_http.py; the two transports retain their different private-network contracts. """ import base64 import functools @@ -36,11 +35,14 @@ import os import re import socket +import time import urllib.error import urllib.request from urllib.parse import urlparse HTTP_TIMEOUT = 12 +MAX_RESPONSE_BYTES = 4 * 1024 * 1024 +MAX_ERROR_BYTES = 64 * 1024 _SM = None _SECRET_CACHE = None @@ -62,6 +64,46 @@ class SsrfBlocked(Exception): """Raised when an endpoint host/scheme is disallowed.""" +class ResponseTooLarge(ValueError): + """The upstream body exceeded the read budget; this is never an empty result.""" + + +def _seconds(value, default, ceiling=55): + if isinstance(value, bool): + return default + text = str(value).strip() + if not re.fullmatch(r"\d+s?", text): + return default + return max(1, min(ceiling, int(text.removesuffix("s")))) + + +def request_timeout(creds): + """Saved transport deadline for APIs without a provider execution-time parameter.""" + return _seconds(creds.get("timeoutS"), HTTP_TIMEOUT) + + +def execution_timeout(creds, requested=None): + """Provider execution ceiling, with a caller allowed only to tighten it.""" + configured = _seconds(creds.get("timeoutS"), 10) + return min(configured, _seconds(requested, configured)) + + +def finite_number(value, bound=1.7976931348623157e308): + """JSON chart numbers exclude booleans, non-finite values and oversized integers.""" + return type(value) in (int, float) and abs(value) <= bound + + +class ObservedPayload(dict): + """Transport evidence is an attribute, so upstream JSON cannot impersonate it.""" + def __init__(self, data, status): + super().__init__(data) + self.http_status = status + + +def observed_payload(data, status): + return ObservedPayload(data, status) if isinstance(data, dict) else data + + # Cloud instance-metadata endpoints blocked even though private is otherwise allowed (mirror agent.py). _METADATA_IPS = frozenset({ ipaddress.ip_address("169.254.169.254"), @@ -230,15 +272,18 @@ def load_datasource(slug, instance_id=None): 1. the request-scoped inline conn-config (`_REQUEST_CONN`) — the trusted BFF path only; 2. the per-instance secret key `map[str(instance_id)]` — the credential-blind worker path, where the caller sends ONLY an instance_id and the connector reads the secret server-side; - 3. the `map[slug]` kind-mirror (the default instance) as the fallback. + 3. the `map[slug]` kind-mirror, only when no explicit instance was requested. `instance_id` is a pure argument (no module-global), so warm-container reuse cannot bleed one invocation's instance into the next.""" if _REQUEST_CONN is not None: return _REQUEST_CONN m = _load_secret_map() creds = m.get(str(instance_id)) if instance_id is not None else None - if not (isinstance(creds, dict) and creds.get("endpoint")): - creds = m.get(slug) # kind-mirror (default instance) fallback + if instance_id is not None: + if not isinstance(creds, dict) or not creds.get("endpoint"): + raise NotConnected("requested datasource instance is not configured") + else: + creds = m.get(slug) if not isinstance(creds, dict) or not creds.get("endpoint"): raise NotConnected(f"{slug} not connected (no endpoint configured in the Connectors UI)") return creds @@ -341,7 +386,28 @@ def _parse(raw): return {"raw": raw.decode("utf-8", "replace")[:4000]} -def http_json(method, url, headers=None, body=None, timeout=HTTP_TIMEOUT): +def _read_bounded(response, max_bytes, timeout): + """Read at most budget+1 bytes; never trust Content-Length or parse before the bound.""" + chunks = [] + total = 0 + deadline = time.monotonic() + timeout + read = getattr(response, "read1", response.read) + try: + while True: + if time.monotonic() >= deadline: + raise TimeoutError("datasource response read deadline exceeded") + chunk = read(min(65536, max_bytes - total + 1)) + if not chunk: + return b"".join(chunks) + total += len(chunk) + if total > max_bytes: + raise ResponseTooLarge(f"datasource response exceeds {max_bytes} bytes") + chunks.append(chunk) + finally: + response.close() + + +def http_json(method, url, headers=None, body=None, timeout=HTTP_TIMEOUT, max_bytes=MAX_RESPONSE_BYTES): """Send a request (no auto-redirect). Returns (status, parsed_dict). Non-2xx → (status, body). Pinned to the IPs assert_host_allowed(url) most recently validated for this host, if any (tried in order with fallback) — see module docstring (IP-pinning).""" @@ -349,10 +415,7 @@ def http_json(method, url, headers=None, body=None, timeout=HTTP_TIMEOUT): req = urllib.request.Request(url, data=data, method=method, headers=headers or {}) try: resp = _opener_for(urlparse(url).hostname).open(req, timeout=timeout) - return resp.getcode(), _parse(resp.read()) + return resp.getcode(), _parse(_read_bounded(resp, max_bytes, timeout)) except urllib.error.HTTPError as e: - try: - raw = e.read() - except Exception: # noqa: BLE001 - raw = b"" + raw = _read_bounded(e, min(max_bytes, MAX_ERROR_BYTES), timeout) return e.code, _parse(raw) diff --git a/agent/lambda/dynatrace_mcp.py b/agent/lambda/dynatrace_mcp.py index cb68b58fa..0329530ed 100644 --- a/agent/lambda/dynatrace_mcp.py +++ b/agent/lambda/dynatrace_mcp.py @@ -16,11 +16,12 @@ from cross_account import resolve_tool_name from datasource_http import ( NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, http_json, load_datasource, - set_request_conn, + set_request_conn, request_timeout, finite_number, observed_payload, ) SLUG = "dynatrace" MAX_SERIES = 50 +MAX_POINTS = 1500 _REL = re.compile(r"^(\d+)([smhdw])$") _NATIVE = re.compile(r"^now(-\d+[smhdw])?$") @@ -46,7 +47,7 @@ def _headers(creds): """Api-Token scheme: an explicit token (authType bearer) becomes `Authorization: Api-Token …`; custom-header auth passes through unchanged (auth_headers refuses to set Authorization itself).""" h = dict(auth_headers(creds)) - if creds.get("token"): + if creds.get("token") and creds.get("authType") in (None, "bearer"): h["Authorization"] = f"Api-Token {creds['token']}" return h @@ -59,12 +60,12 @@ def _ds(): def _get(creds, path, params=None): url = creds["endpoint"].rstrip("/") + path + ("?" + urlencode(params, doseq=True) if params else "") - status, data = http_json("GET", url, headers=_headers(creds)) + status, data = http_json("GET", url, headers=_headers(creds), timeout=request_timeout(creds)) if status >= 400: detail = data.get("error") if isinstance(data, dict) else None msg = detail.get("message") if isinstance(detail, dict) else (detail or data.get("raw") if isinstance(data, dict) else data) raise _ApiError(f"Dynatrace HTTP {status}: {str(msg)[:300]}") - return data + return observed_payload(data, status) def dynatrace_query(args): @@ -79,24 +80,46 @@ def dynatrace_query(args): if args.get("resolution"): params["resolution"] = str(args["resolution"]) data = _get(_ds(), "/api/v2/metrics/query", params) - result = data.get("result", []) if isinstance(data, dict) else [] + if isinstance(data, dict) and data.get("error") not in (None, ""): + return ok({"result": [], "truncated": False, "collectionStatus": "error"}) + result = data.get("result") if isinstance(data, dict) else None + if not isinstance(result, list): + return ok({"result": [], "truncated": False, "collectionStatus": "unknown"}) out = [] + # Current Metrics v2 query responses have a null nextPageKey. Retain partial + # evidence if an older/compatible response advertises an unconsumed page. + truncated = len(result) > MAX_SERIES or bool(isinstance(data, dict) and data.get("nextPageKey")) + invalid = False for metric in result[:MAX_SERIES]: - if not isinstance(metric, dict): + if not isinstance(metric, dict) or not isinstance(metric.get("metricId"), str) or not isinstance(metric.get("data"), list): + invalid = True continue + truncated = truncated or len(metric["data"]) > MAX_SERIES + series = [] + for entry in metric["data"][:MAX_SERIES]: + if (not isinstance(entry, dict) or not isinstance(entry.get("dimensions"), list) + or not all(isinstance(value, str) for value in entry["dimensions"]) + or not isinstance(entry.get("timestamps"), list) or not isinstance(entry.get("values"), list) + or len(entry["timestamps"]) != len(entry["values"])): + invalid = True + continue + timestamps, values = [], [] + truncated = truncated or len(entry["values"]) > MAX_POINTS + for timestamp, value in zip(entry["timestamps"][:MAX_POINTS], entry["values"][:MAX_POINTS]): + if (not finite_number(timestamp, 8640000000000000) + or value is not None and not finite_number(value)): + invalid = True + continue + timestamps.append(timestamp) + values.append(value) + series.append({"dimensions": entry["dimensions"], "timestamps": timestamps, "values": values}) out.append({ - "metricId": metric.get("metricId", ""), - "data": [ - { - "dimensions": d.get("dimensions", []), - "timestamps": d.get("timestamps", []), - "values": d.get("values", []), - } - for d in (metric.get("data") or [])[:MAX_SERIES] - if isinstance(d, dict) - ], + "metricId": metric["metricId"], "data": series, }) - return ok({"result": out, "truncated": len(result) > MAX_SERIES, + http_status = getattr(data, "http_status", 200) + state = "unknown" if invalid or http_status not in (200, 206) else "partial" if truncated or http_status == 206 else "ok" if out else "empty" + return ok({"result": out, "truncated": truncated, + "collectionStatus": state, "resolution": data.get("resolution") if isinstance(data, dict) else None}) @@ -125,14 +148,19 @@ def dynatrace_schema(args): def dynatrace_health(args): - """Connectivity + token probe: metrics list pageSize=1 (requires metrics.read scope).""" + """Verify Metrics v2 query access with a one-minute, one-resolution query.""" creds = load_datasource(SLUG) assert_host_allowed(creds["endpoint"]) try: - _get(creds, "/api/v2/metrics", {"pageSize": "1"}) - return ok({"ok": True}) - except _ApiError as e: - return ok({"ok": False, "error": str(e)}) + result = json.loads(dynatrace_query({ + "query": "builtin:host.cpu.usage:avg", "start": "now-1m", "end": "now", + "resolution": "1m", + })["body"]) + verified = result.get("collectionStatus") in ("ok", "empty", "partial") + return ok({"ok": verified, "verified": ["query"] if verified else [], + **({} if verified else {"error": "Dynatrace metric-query access could not be verified"})}) + except Exception: + return ok({"ok": False, "error": "Dynatrace metric-query access could not be verified"}) _TOOLS = { diff --git a/agent/lambda/jaeger_mcp.py b/agent/lambda/jaeger_mcp.py index 7edf3dedc..d9b1187c7 100644 --- a/agent/lambda/jaeger_mcp.py +++ b/agent/lambda/jaeger_mcp.py @@ -18,7 +18,7 @@ from cross_account import resolve_tool_name from datasource_http import ( NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, health, http_json, load_datasource, - set_request_conn, + set_request_conn, request_timeout, observed_payload, ) SLUG = "jaeger" @@ -54,10 +54,10 @@ def _ds(): def _get(creds, path, params=None): url = creds["endpoint"].rstrip("/") + path + ("?" + urlencode(params, doseq=True) if params else "") - status, data = http_json("GET", url, headers=auth_headers(creds)) + status, data = http_json("GET", url, headers=auth_headers(creds), timeout=request_timeout(creds)) if status >= 400: # Jaeger errors carry {"errors":[...]} — success = HTTP 2xx raise _ApiError(f"Jaeger HTTP {status}: {str(data.get('errors') or data.get('raw') or data)[:300]}") - return data + return observed_payload(data, status) def _byte_bound(obj): @@ -88,16 +88,34 @@ def jaeger_search(args): except _ApiError as e: return err(str(e)) params.setdefault("limit", "20") - params["limit"] = str(min(int(params["limit"]), MAX_TRACES)) + if not re.fullmatch(r"\d{1,9}", str(params["limit"])): + return err("limit must be a positive integer") + requested = max(1, min(int(params["limit"]), MAX_TRACES)) + params["limit"] = str(requested) params["start"] = _parse_time_us(args.get("start"), 3600) params["end"] = _parse_time_us(args.get("end")) data = _get(_ds(), "/api/traces", params) - traces = data.get("data", []) if isinstance(data, dict) else [] + if isinstance(data, dict) and data.get("errors"): + return ok({"traces": [], "truncated": False, "collectionStatus": "error"}) + traces = data.get("data") if isinstance(data, dict) else None + if not isinstance(traces, list): + return ok({"traces": [], "truncated": False, "collectionStatus": "unknown"}) # Compact rows for the console/agent: full span trees are huge — summarize per trace. rows = [] + invalid = False for t in traces[:MAX_TRACES]: - spans = t.get("spans", []) if isinstance(t, dict) else [] - procs = t.get("processes", {}) if isinstance(t, dict) else {} + if (not isinstance(t, dict) or not isinstance(t.get("traceID"), str) + or len(t["traceID"]) not in (16, 32) or not _HEX.fullmatch(t["traceID"]) + or not isinstance(t.get("spans"), list) or not t["spans"] + or not isinstance(t.get("processes"), dict)): + invalid = True + continue + spans, procs = t["spans"], t["processes"] + if not all(isinstance(s, dict) and type(s.get("startTime")) is int and 0 <= s["startTime"] <= 8640000000000000000 + and type(s.get("duration")) is int and 0 <= s["duration"] <= 8640000000000000000 + and isinstance(s.get("operationName"), str) for s in spans): + invalid = True + continue root = min(spans, key=lambda s: s.get("startTime", 0)) if spans else {} dur_us = max((s.get("startTime", 0) + s.get("duration", 0)) for s in spans) - root.get("startTime", 0) if spans else 0 svc = "" @@ -112,8 +130,13 @@ def jaeger_search(args): "durationMs": round(dur_us / 1000, 1), "startTime": root.get("startTime", 0), }) + truncated = len(traces) >= requested + http_status = getattr(data, "http_status", 200) + state = "unknown" if invalid or http_status not in (200, 206) else "partial" if truncated or http_status == 206 else "ok" if rows else "empty" payload, btr = _byte_bound({"traces": rows}) - return ok(payload if btr else {"truncated": len(traces) > MAX_TRACES, **payload}) + return ok({**payload, "truncated": truncated or btr, + "collectionStatus": "unknown" if invalid else "partial" if btr else state, + "requestedLimit": requested, "returnedCount": len(rows)}) def jaeger_get_trace(args): @@ -149,8 +172,18 @@ def jaeger_schema(args): def jaeger_health(args): - """Connectivity probe for the pre-save Test / status badge: GET /api/services.""" - return ok(health(load_datasource(SLUG), "/api/services")) + """Verify a recognizable query response, not an arbitrary HTTP 200.""" + try: + end = int(time.time()) + result = json.loads(jaeger_search({ + "query": "service=awsops-connection-probe&limit=1", + "start": str(end - 60), "end": str(end), + })["body"]) + verified = result.get("collectionStatus") in ("ok", "empty", "partial") + return ok({"ok": verified, "verified": ["query"] if verified else [], + **({} if verified else {"error": "Jaeger query access could not be verified"})}) + except Exception: + return ok({"ok": False, "error": "Jaeger query access could not be verified"}) _TOOLS = { diff --git a/agent/lambda/loki_mcp.py b/agent/lambda/loki_mcp.py index a514fdf7b..7f9693c32 100644 --- a/agent/lambda/loki_mcp.py +++ b/agent/lambda/loki_mcp.py @@ -16,7 +16,7 @@ from cross_account import resolve_tool_name from datasource_http import ( NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, health, http_json, load_datasource, - set_request_conn, + set_request_conn, request_timeout, observed_payload, finite_number, ) SLUG = "loki" @@ -64,12 +64,68 @@ def _ds(): def _get(creds, path, params): url = creds["endpoint"].rstrip("/") + path + ("?" + urlencode(params, doseq=True) if params else "") - status, data = http_json("GET", url, headers=_headers(creds)) + status, data = http_json("GET", url, headers=_headers(creds), timeout=request_timeout(creds)) if status >= 400: raise _ApiError(f"Loki HTTP {status}: {str(data.get('raw') or data.get('error') or data)[:300]}") if isinstance(data, dict) and data.get("status") and data.get("status") != "success": raise _ApiError(f"Loki query failed: {data.get('error', 'unknown')}") - return data.get("data") if isinstance(data, dict) else data + if not isinstance(data, dict) or data.get("status") != "success" or "data" not in data: + raise _ApiError("Loki response could not be verified") + result = observed_payload(data["data"], status) + if isinstance(result, dict): + result.source_partial = bool(data.get("warnings") or data.get("partial") or data.get("truncated")) + result.source_unverified = ( + "warnings" in data and (not isinstance(data["warnings"], list) + or not all(isinstance(value, str) for value in data["warnings"])) + or any(key in data and type(data[key]) is not bool for key in ("partial", "truncated")) + ) + return result + + +def _valid_row(row, kind): + if not isinstance(row, dict): + return False + labels = row.get("stream" if kind == "streams" else "metric") + if not isinstance(labels, dict) or not all(isinstance(key, str) and isinstance(value, str) for key, value in labels.items()): + return False + samples = row.get("values") if kind != "vector" else [row.get("value")] + if not isinstance(samples, list): + return False + for pair in samples[:MAX_LINES_PER_STREAM]: + if not isinstance(pair, list) or len(pair) != 2 or type(pair[0]) not in (str, int, float): + return False + try: + if not finite_number(float(pair[0]), 8640000000000000000000 if kind == "streams" else 8640000000000): + return False + except (ValueError, OverflowError): + return False + if kind == "streams": + if not isinstance(pair[1], str): + return False + elif (type(pair[1]) not in (str, int, float) + or isinstance(pair[1], str) and not re.fullmatch(r"(?:NaN|[+-]?Inf|[+-]?(?:\d+(?:\.\d*)?|\.\d+)(?:[eE][+-]?\d+)?)", pair[1]) + or not isinstance(pair[1], str) and not finite_number(pair[1])): + return False + return True + + +def _query_response(data): + valid = (isinstance(data, dict) and data.get("resultType") in ("streams", "matrix", "vector") + and isinstance(data.get("result"), list) and not getattr(data, "source_unverified", False)) + kind = data.get("resultType") if isinstance(data, dict) else None + source_rows = data.get("result") if isinstance(data, dict) else None + source_rows = source_rows if isinstance(source_rows, list) else [] + retained = source_rows[:MAX_STREAMS] + rows = [row for row in retained if _valid_row(row, kind)] if kind in ("streams", "matrix", "vector") else [] + valid = valid and len(rows) == len(retained) + bounded, truncated = _bound({"resultType": kind, "result": rows}) + truncated = truncated or len(source_rows) > MAX_STREAMS + status = getattr(data, "http_status", 200) + state = ("unknown" if not valid or status not in (200, 206) else + "partial" if truncated or status == 206 or getattr(data, "source_partial", False) else + "ok" if data["result"] else "empty") + return ok({"truncated": truncated, **(bounded if isinstance(bounded, dict) else {"result": None}), + "collectionStatus": state}) def _bound(data): @@ -116,8 +172,7 @@ def loki_query_range(args): if args.get("step"): params["step"] = str(args["step"]) data = _get(_ds(), "/loki/api/v1/query_range", params) - bounded, tr = _bound(data) - return ok({"truncated": tr, **(bounded if isinstance(bounded, dict) else {"result": bounded})}) + return _query_response(data) def loki_query(args): @@ -128,8 +183,7 @@ def loki_query(args): if args.get("time"): params["time"] = _parse_time_ns(args.get("time")) data = _get(_ds(), "/loki/api/v1/query", params) - bounded, tr = _bound(data) - return ok({"truncated": tr, **(bounded if isinstance(bounded, dict) else {"result": bounded})}) + return _query_response(data) def loki_labels(args): @@ -173,8 +227,22 @@ def loki_schema(args): def loki_health(args): - """Connectivity probe for the pre-save Test / status badge: GET /ready.""" - return ok(health(load_datasource(SLUG), "/ready")) + """Verify the tenant-scoped query endpoint with a one-minute, one-line search.""" + try: + end = int(time.time()) + result = _get(_ds(), "/loki/api/v1/query_range", { + "query": '{service_name="awsops-connection-probe"}', + "start": str((end - 60) * 1_000_000_000), "end": str(end * 1_000_000_000), + "limit": "1", + }) + verified = (isinstance(result, dict) and result.get("resultType") == "streams" + and isinstance(result.get("result"), list) + and all(isinstance(row, dict) and isinstance(row.get("values"), list) + for row in result["result"])) + return ok({"ok": verified, "verified": ["query"] if verified else [], + **({} if verified else {"error": "Loki tenant query access could not be verified"})}) + except Exception: + return ok({"ok": False, "error": "Loki tenant query access could not be verified"}) _TOOLS["loki_health"] = loki_health diff --git a/agent/lambda/mimir_mcp.py b/agent/lambda/mimir_mcp.py index 9c8746c17..422a3537f 100644 --- a/agent/lambda/mimir_mcp.py +++ b/agent/lambda/mimir_mcp.py @@ -15,6 +15,7 @@ from datasource_http import ( NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, health, http_json, load_datasource, set_request_conn, + execution_timeout, request_timeout, ) SLUG = "mimir" @@ -61,8 +62,18 @@ def _ds(): def _get(creds, path, params, http_timeout=None, *, with_status=False): + if path.startswith(BASE): + prefix = creds.get("apiPrefix", "/prometheus") + if not isinstance(prefix, str) or len(prefix) > 128 or not re.fullmatch(r"(?:/[A-Za-z0-9_-]+)*", prefix): + raise _ApiError("invalid Mimir API prefix") + # An API base already ending in the configured prefix must not duplicate it. + from urllib.parse import urlparse + endpoint_path = urlparse(creds["endpoint"]).path.rstrip("/") + if prefix and endpoint_path.endswith(prefix): + prefix = "" + path = prefix + "/api/v1" + path[len(BASE):] url = creds["endpoint"].rstrip("/") + path + ("?" + urlencode(params, doseq=True) if params else "") - kwargs = {"headers": _headers(creds)} + kwargs = {"headers": _headers(creds), "timeout": min(12, request_timeout(creds))} if http_timeout is not None: kwargs["timeout"] = http_timeout status, data = http_json("GET", url, **kwargs) @@ -204,10 +215,10 @@ def mimir_query(args): if not query: return err("query (PromQL) required") params = {"query": query, "time": _parse_time(args.get("time"))} - timeout = _timeout_param(args.get("timeout")) - if timeout: - params["timeout"] = timeout - return _query_result(_get(_ds(), f"{BASE}/query", params, with_status=True), allow_scalar=True) + creds = _ds() + timeout = execution_timeout(creds, args.get("timeout")) + params["timeout"] = f"{timeout}s" + return _query_result(_get(creds, f"{BASE}/query", params, http_timeout=timeout + 3, with_status=True), allow_scalar=True) def mimir_query_range(args): @@ -216,10 +227,10 @@ def mimir_query_range(args): return err("query (PromQL) required") params = {"query": query, "start": _parse_time(args.get("start"), 3600), "end": _parse_time(args.get("end")), "step": str(args.get("step") or "60")} - timeout = _timeout_param(args.get("timeout")) - if timeout: - params["timeout"] = timeout - return _query_result(_get(_ds(), f"{BASE}/query_range", params, with_status=True)) + creds = _ds() + timeout = execution_timeout(creds, args.get("timeout")) + params["timeout"] = f"{timeout}s" + return _query_result(_get(creds, f"{BASE}/query_range", params, http_timeout=timeout + 3, with_status=True)) def _list_result(observed, field, limit, kind): @@ -346,8 +357,14 @@ def mimir_metric_meta(args): def mimir_health(args): - """Connectivity probe for the pre-save Test / status badge: GET /ready.""" - return ok(health(load_datasource(SLUG), "/ready")) + """Verify tenant-scoped query access, independently of process readiness.""" + try: + result = json.loads(mimir_query({"query": "vector(1)", "timeout": "1s"})["body"]) + verified = result.get("collectionStatus") == "ok" + return ok({"ok": verified, "verified": ["query"] if verified else [], + **({} if verified else {"error": "Mimir tenant query access could not be verified"})}) + except Exception: + return ok({"ok": False, "error": "Mimir tenant query access could not be verified"}) _TOOLS["mimir_health"] = mimir_health diff --git a/agent/lambda/prometheus_mcp.py b/agent/lambda/prometheus_mcp.py index 1cc79f1bd..62011a10a 100644 --- a/agent/lambda/prometheus_mcp.py +++ b/agent/lambda/prometheus_mcp.py @@ -25,6 +25,8 @@ http_json, load_datasource, set_request_conn, + execution_timeout, + request_timeout, ) SLUG = "prometheus" @@ -67,7 +69,7 @@ def _ds(): def _get(creds, path, params, http_timeout=None, *, with_status=False): url = creds["endpoint"].rstrip("/") + path + ("?" + urlencode(params, doseq=True) if params else "") - kwargs = {"headers": auth_headers(creds)} + kwargs = {"headers": auth_headers(creds), "timeout": min(12, request_timeout(creds))} if http_timeout is not None: kwargs["timeout"] = http_timeout status, data = http_json("GET", url, **kwargs) @@ -212,10 +214,10 @@ def prometheus_query(args): if not query: return err("query (PromQL) required") params = {"query": query, "time": _parse_time(args.get("time"))} - timeout = _timeout_param(args.get("timeout")) - if timeout: - params["timeout"] = timeout - return _query_result(_get(_ds(), "/api/v1/query", params, with_status=True), allow_scalar=True) + creds = _ds() + timeout = execution_timeout(creds, args.get("timeout")) + params["timeout"] = f"{timeout}s" + return _query_result(_get(creds, "/api/v1/query", params, http_timeout=timeout + 3, with_status=True), allow_scalar=True) def prometheus_query_range(args): @@ -226,10 +228,10 @@ def prometheus_query_range(args): end = _parse_time(args.get("end")) step = str(args.get("step") or "60").strip() params = {"query": query, "start": start, "end": end, "step": step} - timeout = _timeout_param(args.get("timeout")) - if timeout: - params["timeout"] = timeout - return _query_result(_get(_ds(), "/api/v1/query_range", params, with_status=True)) + creds = _ds() + timeout = execution_timeout(creds, args.get("timeout")) + params["timeout"] = f"{timeout}s" + return _query_result(_get(creds, "/api/v1/query_range", params, http_timeout=timeout + 3, with_status=True)) def _list_result(observed, field, limit, kind): @@ -297,8 +299,14 @@ def prometheus_schema(args): def prometheus_health(args): - """Connectivity probe for the pre-save Test / status badge: GET /-/healthy.""" - return ok(health(_ds(), "/-/healthy")) + """Verify the actual authenticated query API using a bounded scalar query.""" + try: + result = json.loads(prometheus_query({"query": "vector(1)", "timeout": "1s"})["body"]) + verified = result.get("collectionStatus") == "ok" + return ok({"ok": verified, "verified": ["query"] if verified else [], + **({} if verified else {"error": "Prometheus query access could not be verified"})}) + except Exception: + return ok({"ok": False, "error": "Prometheus query access could not be verified"}) def prometheus_metric_meta(args): diff --git a/agent/lambda/tempo_mcp.py b/agent/lambda/tempo_mcp.py index a3672ec87..3aabed856 100644 --- a/agent/lambda/tempo_mcp.py +++ b/agent/lambda/tempo_mcp.py @@ -19,7 +19,7 @@ from cross_account import resolve_tool_name from datasource_http import ( NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, health, http_json, load_datasource, - set_request_conn, + set_request_conn, request_timeout, ) SLUG = "tempo" @@ -78,7 +78,7 @@ def _ds(): def _get(creds, path, params=None, *, timeout=None, with_status=False): url = creds["endpoint"].rstrip("/") + path + ("?" + urlencode(params, doseq=True) if params else "") - request_options = {"timeout": timeout} if timeout is not None else {} + request_options = {"timeout": min(timeout, request_timeout(creds)) if timeout is not None else request_timeout(creds)} status, data = http_json("GET", url, headers=_headers(creds), **request_options) if status >= 400: # Tempo has no envelope status → HTTP 2xx is success detail = (data.get("raw") or data.get("error") or data) if isinstance(data, dict) else "non-object error response" @@ -542,8 +542,18 @@ def tempo_schema(args): def tempo_health(args): - """Connectivity probe for the pre-save Test / status badge: GET /ready.""" - return ok(health(load_datasource(SLUG), "/ready")) + """Verify query access using a one-minute, one-trace search, not process readiness.""" + try: + end = int(time.time()) + result = json.loads(tempo_search({ + "query": '{ resource.service.name = "awsops-connection-probe" }', + "start": str(end - 60), "end": str(end), "limit": "1", + })["body"]) + verified = result.get("collectionStatus") in ("ok", "empty", "partial") + return ok({"ok": verified, "verified": ["query"] if verified else [], + **({} if verified else {"error": "Tempo tenant query access could not be verified"})}) + except Exception: + return ok({"ok": False, "error": "Tempo tenant query access could not be verified"}) _TOOLS["tempo_health"] = tempo_health diff --git a/agent/lambda/test_datadog_mcp.py b/agent/lambda/test_datadog_mcp.py new file mode 100644 index 000000000..5c9535e2c --- /dev/null +++ b/agent/lambda/test_datadog_mcp.py @@ -0,0 +1,81 @@ +"""Datadog query-access and result-loss regressions; provider transport is mocked.""" +import json +import sys +from pathlib import Path +from unittest.mock import patch + +sys.path.insert(0, str(Path(__file__).parent)) +import datadog_mcp as dd + + +def body(value): + return json.loads(value["body"]) + + +CREDS = {"endpoint": "https://provider.example.test", "apiKey": "synthetic-api", "appKey": "synthetic-app"} + + +def test_api_key_validation_alone_cannot_pass_when_metric_query_is_denied(): + with patch.object(dd, "load_datasource", return_value=CREDS), patch.object(dd, "assert_host_allowed"), \ + patch.object(dd, "_get", side_effect=[{"valid": True}, dd._ApiError("synthetic-app denied")]) as request: + result = body(dd.datadog_health({})) + assert result["ok"] is False + assert request.call_args_list[1].args[1] == "/api/v1/query" + assert "synthetic-app" not in json.dumps(result) + + +def test_health_accepts_a_successfully_queried_empty_probe_and_bounds_its_window(): + with patch.object(dd, "load_datasource", return_value=CREDS), patch.object(dd, "assert_host_allowed"), \ + patch.object(dd.time, "time", return_value=1000), \ + patch.object(dd, "_get", side_effect=[{"valid": True}, {"status": "ok", "series": []}]) as request: + assert body(dd.datadog_health({})) == {"ok": True} + params = request.call_args_list[1].args[2] + assert params["from"] == "940" and params["to"] == "1000" + assert "{*}" not in params["query"] + + +def test_invalid_api_key_never_starts_a_metric_probe(): + with patch.object(dd, "load_datasource", return_value=CREDS), patch.object(dd, "assert_host_allowed"), \ + patch.object(dd, "_get", return_value={"valid": False}) as request: + assert body(dd.datadog_health({}))["ok"] is False + request.assert_called_once() + + +def test_health_does_not_accept_an_unverified_query_envelope(): + with patch.object(dd, "load_datasource", return_value=CREDS), patch.object(dd, "assert_host_allowed"), \ + patch.object(dd, "_get", side_effect=[{"valid": True}, {"status": "error", "series": []}]): + assert body(dd.datadog_health({}))["ok"] is False + + +def test_bearer_connection_tests_the_metric_operation_without_api_key_validation(): + creds = {"endpoint": CREDS["endpoint"], "authType": "bearer", "token": "synthetic-bearer"} + with patch.object(dd, "load_datasource", return_value=creds), patch.object(dd, "assert_host_allowed"), \ + patch.object(dd, "_get", return_value={"status": "ok", "series": []}) as request: + assert body(dd.datadog_health({})) == {"ok": True} + request.assert_called_once() + assert request.call_args.args[1] == "/api/v1/query" + + +def test_point_limit_is_disclosed_and_preserves_other_series(): + points = [[1000 + i, i] for i in range(dd.MAX_POINTS + 1)] + response = {"status": "ok", "series": [ + {"metric": "large", "pointlist": points}, {"metric": "small", "pointlist": [[1000, 0]]}, + ]} + with patch.object(dd, "_ds", return_value=CREDS), patch.object(dd, "_get", return_value=response): + result = body(dd.datadog_query({"query": "avg:fixture.metric{*}"})) + assert len(result["series"][0]["pointlist"]) == dd.MAX_POINTS + assert result["series"][1]["pointlist"] == [[1000, 0]] + assert result["truncated"] is True and result["collectionStatus"] == "partial" + assert len(response["series"][0]["pointlist"]) == dd.MAX_POINTS + 1 + + +def test_outer_series_limit_is_disclosed_and_an_exact_point_limit_is_not(): + with patch.object(dd, "_ds", return_value=CREDS), patch.object(dd, "_get", return_value={ + "status": "ok", "series": [{"metric": str(i), "pointlist": [[1000, 1]]} for i in range(dd.MAX_SERIES + 1)], + }): + result = body(dd.datadog_query({"query": "avg:fixture.metric{*}"})) + assert len(result["series"]) == dd.MAX_SERIES and result["truncated"] is True + with patch.object(dd, "_ds", return_value=CREDS), patch.object(dd, "_get", return_value={ + "status": "ok", "series": [{"metric": "exact", "pointlist": [[1000, 1]] * dd.MAX_POINTS}], + }): + assert body(dd.datadog_query({"query": "avg:fixture.metric{*}"}))["truncated"] is False diff --git a/agent/lambda/test_datasource_http.py b/agent/lambda/test_datasource_http.py index f408ac75b..569cc66ab 100644 --- a/agent/lambda/test_datasource_http.py +++ b/agent/lambda/test_datasource_http.py @@ -90,7 +90,8 @@ def test_load_datasource_instance_id_pure_arg(self): with mock.patch.object(dh, "_load_secret_map", return_value=m): self.assertEqual(dh.load_datasource("prometheus", instance_id=5)["endpoint"], "http://prom-a:9090") self.assertEqual(dh.load_datasource("prometheus")["endpoint"], "http://prom-default:9090") # kind mirror - self.assertEqual(dh.load_datasource("prometheus", instance_id=999)["endpoint"], "http://prom-default:9090") # missing id → mirror + with self.assertRaises(dh.NotConnected): + dh.load_datasource("prometheus", instance_id=999) def test_warm_container_no_bleed(self): # an inline conn set on one invocation must not bleed into a later instance_id invocation. @@ -115,10 +116,14 @@ def test_no_redirect_follow(self): class _R: def __init__(self, code): self._c = code + from io import BytesIO + self.body = BytesIO(b'{"ok":1}') def getcode(self): return self._c - def read(self): - return b'{"ok":1}' + def read(self, size=-1): + return self.body.read(size) + def close(self): + self.body.close() with mock.patch.object(dh._opener, "open", return_value=_R(200)): status, data = dh.http_json("GET", "http://ch:8123/ping") self.assertEqual(status, 200) diff --git a/agent/lambda/test_dynatrace_mcp.py b/agent/lambda/test_dynatrace_mcp.py new file mode 100644 index 000000000..0448c155c --- /dev/null +++ b/agent/lambda/test_dynatrace_mcp.py @@ -0,0 +1,46 @@ +"""Dynatrace nested caps and advertised page-loss regressions; no live provider.""" +import json +import sys +from pathlib import Path +from unittest.mock import patch + +import pytest + +sys.path.insert(0, str(Path(__file__).parent)) +import dynatrace_mcp as dt + + +def query(response): + with patch.object(dt, "_ds", return_value={"endpoint": "https://provider.example.test"}), \ + patch.object(dt, "_get", return_value=response) as request: + result = json.loads(dt.dynatrace_query({"query": "fixture.metric"})["body"]) + request.assert_called_once() + return result + + +def series(count): + return [{"dimensions": [str(i)], "timestamps": [1000], "values": [i]} for i in range(count)] + + +def test_nested_dimension_loss_is_partial_even_for_one_metric(): + response = {"result": [{"metricId": "fixture.metric", "data": series(dt.MAX_SERIES + 1)}]} + result = query(response) + assert len(result["result"][0]["data"]) == dt.MAX_SERIES + assert result["truncated"] is True and result["collectionStatus"] == "partial" + assert len(response["result"][0]["data"]) == dt.MAX_SERIES + 1 + + +def test_outer_metric_cap_is_disclosed(): + result = query({"result": [{"metricId": str(i), "data": series(1)} for i in range(dt.MAX_SERIES + 1)]}) + assert len(result["result"]) == dt.MAX_SERIES and result["truncated"] is True + + +def test_unconsumed_compatibility_page_is_partial_and_is_not_automatically_fetched(): + result = query({"result": [{"metricId": "fixture.metric", "data": series(1)}], "nextPageKey": "next-fixture"}) + assert result["truncated"] is True and result["collectionStatus"] == "partial" + + +@pytest.mark.parametrize("page", [None, ""]) +def test_exact_dimension_cap_and_no_next_page_are_not_loss(page): + result = query({"result": [{"metricId": "fixture.metric", "data": series(dt.MAX_SERIES)}], "nextPageKey": page}) + assert result["truncated"] is False diff --git a/agent/lambda/test_external_connections.py b/agent/lambda/test_external_connections.py new file mode 100644 index 000000000..37ee517d8 --- /dev/null +++ b/agent/lambda/test_external_connections.py @@ -0,0 +1,229 @@ +"""Connection identity, bounded I/O, query proof and result-completion regressions.""" +import importlib +import io +import json +import sys +import urllib.error +from pathlib import Path +from unittest.mock import patch +from urllib.parse import parse_qs, urlparse + +import pytest + +sys.path.insert(0, str(Path(__file__).parent)) +import datasource_http as dh + +CREDS = {"endpoint": "https://provider.example.test", "authType": "none", "org_id": "synthetic-tenant"} + + +@pytest.fixture(autouse=True) +def reset_request(): + dh.set_request_conn(None) + yield + dh.set_request_conn(None) + + +class Response(io.BytesIO): + def getcode(self): + return 200 + + +@pytest.mark.parametrize("size", [63, 64, 65, 1024]) +def test_byte_bound_precedes_parse_and_closes_success_stream(size): + response = Response(b"x" * size) + with patch.object(dh._opener, "open", return_value=response): + if size > 64: + with pytest.raises(dh.ResponseTooLarge): + dh.http_json("GET", CREDS["endpoint"], max_bytes=64) + else: + assert dh.http_json("GET", CREDS["endpoint"], max_bytes=64)[0] == 200 + assert response.closed + + +def test_error_body_is_bounded_and_closed_without_echoing_content(): + stream = io.BytesIO(b"synthetic-private" * 10000) + error = urllib.error.HTTPError(CREDS["endpoint"], 403, "denied", {}, stream) + with patch.object(dh._opener, "open", side_effect=error), pytest.raises(dh.ResponseTooLarge) as exc: + dh.http_json("GET", CREDS["endpoint"]) + assert stream.closed + assert "synthetic-private" not in str(exc.value) + + +@pytest.mark.parametrize("entry", [None, {}, {"token": "synthetic"}, []]) +def test_explicit_missing_instance_cannot_borrow_default_credentials(entry): + with patch.object(dh, "_load_secret_map", return_value={"7": entry, "prometheus": CREDS}): + with pytest.raises(dh.NotConnected): + dh.load_datasource("prometheus", instance_id=7) + assert dh.load_datasource("prometheus") == CREDS + + +@pytest.mark.parametrize("kind", ["prometheus", "mimir"]) +@pytest.mark.parametrize("configured,requested,expected", [(1, None, 1), (35, None, 35), (35, "5s", 5), (1, "60s", 1), (60, None, 55), (None, None, 10)]) +def test_provider_ceiling_applies_on_default_and_explicit_paths(kind, configured, requested, expected): + module = importlib.import_module(kind + "_mcp") + credentials = {**CREDS, "timeoutS": configured} + response = {"status": "success", "data": {"resultType": "vector", "result": []}} + with patch.object(module, "load_datasource", return_value=credentials), \ + patch.object(module, "assert_host_allowed"), patch.object(module, "http_json", return_value=(200, response)) as http: + arguments = {"query": "up", **({"timeout": requested} if requested is not None else {})} + getattr(module, kind + "_query")(arguments) + assert parse_qs(urlparse(http.call_args.args[1]).query)["timeout"] == [f"{expected}s"] + assert http.call_args.kwargs["timeout"] == expected + 3 + + +@pytest.mark.parametrize("endpoint,prefix,path", [ + ("https://provider.example.test", None, "/prometheus/api/v1/query"), + ("https://provider.example.test/prometheus", None, "/prometheus/api/v1/query"), + ("https://provider.example.test/mount", "/metrics", "/mount/metrics/api/v1/query"), + ("https://provider.example.test", "", "/api/v1/query"), +]) +def test_mimir_mount_is_applied_once(endpoint, prefix, path): + module = importlib.import_module("mimir_mcp") + credentials = {**CREDS, "endpoint": endpoint, **({"apiPrefix": prefix} if prefix is not None else {})} + with patch.object(module, "load_datasource", return_value=credentials), \ + patch.object(module, "assert_host_allowed"), patch.object(module, "http_json", return_value=( + 200, {"status": "success", "data": {"resultType": "vector", "result": []}} + )) as http: + module.mimir_query({"query": "up"}) + assert urlparse(http.call_args.args[1]).path == path + + +@pytest.mark.parametrize("kind", ["prometheus", "mimir", "loki", "tempo", "jaeger", "dynatrace"]) +def test_connection_probe_cannot_pass_with_public_ready_and_denied_query(kind): + module = importlib.import_module(kind + "_mcp") + def upstream(method, url, **kwargs): + return (200, {}) if "/ready" in url or "/healthy" in url else (403, {"error": "synthetic denied"}) + with patch.object(module, "load_datasource", return_value=CREDS), \ + patch.object(module, "assert_host_allowed"), patch.object(module, "http_json", side_effect=upstream): + result = json.loads(getattr(module, kind + "_health")({})["body"]) + assert result["ok"] is False + assert "synthetic denied" not in json.dumps(result) + + +@pytest.mark.parametrize("kind,response", [ + ("prometheus", {"status": "success", "data": {"resultType": "vector", "result": [{"metric": {}, "value": [1, "1"]}]}}), + ("mimir", {"status": "success", "data": {"resultType": "vector", "result": [{"metric": {}, "value": [1, "1"]}]}}), + ("loki", {"status": "success", "data": {"resultType": "streams", "result": []}}), + ("tempo", {"traces": [], "metrics": {}}), + ("jaeger", {"data": []}), + ("dynatrace", {"result": []}), +]) +def test_query_probe_accepts_recognizable_success_and_uses_bounded_query(kind, response): + module = importlib.import_module(kind + "_mcp") + with patch.object(module, "load_datasource", return_value=CREDS), \ + patch.object(module, "assert_host_allowed"), patch.object(module, "http_json", return_value=(200, response)) as http: + result = json.loads(getattr(module, kind + "_health")({})["body"]) + assert result["ok"] is True + assert result["verified"] == ["query"] + url = http.call_args.args[1] + assert "/ready" not in url and "/healthy" not in url + assert http.call_args.kwargs["headers"]["X-Scope-OrgID"] == CREDS["org_id"] + + +@pytest.mark.parametrize("kind", ["jaeger", "dynatrace", "datadog"]) +def test_login_html_is_unknown_not_empty(kind): + module = importlib.import_module(kind + "_mcp") + with patch.object(module, "_ds", return_value=CREDS), patch.object(module, "http_json", return_value=(200, {"raw": "login"})): + query = getattr(module, kind + ("_search" if kind == "jaeger" else "_query")) + result = json.loads(query({"query": "synthetic"})["body"]) + assert result["collectionStatus"] == "unknown" + assert "login" not in json.dumps(result) + + +@pytest.mark.parametrize("kind,response", [ + ("dynatrace", {"result": [{"metricId": "valid", "data": [{"dimensions": [], "timestamps": [1000, 2000], "values": [1]}]}]}), + ("datadog", {"status": "ok", "series": [{"metric": "valid", "pointlist": [[1000, 1], ["invalid", 2]]}]}), +]) +def test_malformed_samples_cannot_certify_complete_metrics(kind, response): + module = importlib.import_module(kind + "_mcp") + with patch.object(module, "_ds", return_value=CREDS), patch.object(module, "_get", return_value=response): + result = json.loads(getattr(module, kind + "_query")({"query": "synthetic"})["body"]) + assert result["collectionStatus"] == "unknown" + + +def trace(index): + return {"traceID": f"{index + 1:032x}", "spans": [{ + "startTime": 1000000, "duration": 100, "operationName": "query", "processID": "p", + }], "processes": {"p": {"serviceName": "synthetic"}}} + + +@pytest.mark.parametrize("count,limit,state", [(0, 20, "empty"), (19, 20, "ok"), (20, 20, "partial"), (1, 1, "partial")]) +def test_jaeger_limit_saturation_is_partial(count, limit, state): + module = importlib.import_module("jaeger_mcp") + with patch.object(module, "_ds", return_value=CREDS), patch.object(module, "_get", return_value={"data": [trace(i) for i in range(count)]}): + result = json.loads(module.jaeger_search({"query": f"service=synthetic&limit={limit}"})["body"]) + assert result["collectionStatus"] == state + assert result["requestedLimit"] == limit and result["returnedCount"] == count + + +@pytest.mark.parametrize("limit", ["-1", "nan", "1.5", "100000000000000000000"]) +def test_jaeger_bad_limit_does_not_reach_provider(limit): + module = importlib.import_module("jaeger_mcp") + with patch.object(module, "_ds") as ds: + assert module.jaeger_search({"query": f"service=synthetic&limit={limit}"})["statusCode"] == 400 + ds.assert_not_called() + + +@pytest.mark.parametrize("value", [float("inf"), float("nan"), True, 10**400]) +@pytest.mark.parametrize("kind", ["dynatrace", "datadog"]) +def test_unrepresentable_chart_numbers_are_unknown_without_serializing_them(kind, value): + module = importlib.import_module(kind + "_mcp") + response = {"result": [{"metricId": "synthetic", "data": [{ + "dimensions": [], "timestamps": [1000], "values": [value], + }]}]} if kind == "dynatrace" else { + "status": "ok", "series": [{"metric": "synthetic", "pointlist": [[1000, value]]}], + } + with patch.object(module, "_ds", return_value=CREDS), patch.object(module, "_get", return_value=response): + result = json.loads(getattr(module, kind + "_query")({"query": "synthetic"})["body"]) + assert result["collectionStatus"] == "unknown" + + +@pytest.mark.parametrize("status,response,verified", [ + (403, {"error": "synthetic password should not be echoed"}, False), + (200, {"raw": "login"}, False), + (200, {"meta": [{"name": "probe", "type": "UInt8"}], "data": [{"probe": 1}], "rows": 1}, True), +]) +def test_clickhouse_probe_verifies_sql_not_ping(status, response, verified): + module = importlib.import_module("clickhouse_mcp") + with patch.object(module, "load_datasource", return_value=CREDS), \ + patch.object(module, "assert_host_allowed"), patch.object(module, "http_json", return_value=(status, response)) as http: + result = json.loads(module.clickhouse_health({})["body"]) + assert result["ok"] is verified + assert "/ping" not in http.call_args.args[1] + assert "max_execution_time=1" in http.call_args.args[1] + assert "synthetic password" not in json.dumps(result) + + +@pytest.mark.parametrize("auth,expected", [ + ("none", {}), (None, {"DD-API-KEY": "synthetic-api", "DD-APPLICATION-KEY": "synthetic-app"}), +]) +def test_explicit_no_auth_cannot_transmit_legacy_datadog_keys(auth, expected): + module = importlib.import_module("datadog_mcp") + credentials = {"apiKey": "synthetic-api", "appKey": "synthetic-app", + **({"authType": auth} if auth is not None else {})} + assert module._headers(credentials) == expected + + +@pytest.mark.parametrize("kind,response", [ + ("datadog", {"status": "ok", "series": [], "http_status": 200}), + ("dynatrace", {"result": [], "http_status": 200}), + ("jaeger", {"data": [], "http_status": 200}), + ("loki", {"status": "success", "data": {"resultType": "streams", "result": []}}), +]) +@pytest.mark.parametrize("status,state", [(200, "empty"), (206, "partial"), (201, "unknown")]) +def test_partial_http_responses_cannot_certify_empty_or_spoof_transport_evidence(kind, response, status, state): + module = importlib.import_module(kind + "_mcp") + with patch.object(module, "_ds", return_value=CREDS), patch.object(module, "http_json", return_value=(status, response)): + query = getattr(module, kind + ("_search" if kind == "jaeger" else "_query")) + result = json.loads(query({"query": "synthetic"})["body"]) + assert result["collectionStatus"] == state + + +def test_loki_malformed_siblings_preserve_valid_logs_as_unverified(): + module = importlib.import_module("loki_mcp") + row = {"stream": {"service_name": "synthetic"}, "values": [["1000000000", "valid log"]]} + response = {"status": "success", "data": {"resultType": "streams", "result": [row, {"values": "invalid"}]}} + with patch.object(module, "_ds", return_value=CREDS), patch.object(module, "http_json", return_value=(200, response)): + result = json.loads(module.loki_query({"query": "synthetic"})["body"]) + assert result["result"] == [row] + assert result["collectionStatus"] == "unknown" diff --git a/agent/lambda/test_mimir_mcp.py b/agent/lambda/test_mimir_mcp.py index 8e089dce6..7b86ad58c 100644 --- a/agent/lambda/test_mimir_mcp.py +++ b/agent/lambda/test_mimir_mcp.py @@ -25,7 +25,7 @@ def test_execution_timeout_is_passed_and_clamped(self): # the connector bounded response SIZE only; a caller can now run model-written PromQL through it, # so the diag-signal dry run asks for an execution bound (review: it was applied to prometheus but # silently ignored here). - for given, expect in (("5s", "5s"), (5, "5s"), ("999", "60s"), ("3600s", "60s"), ("0", "1s"), (0, "1s")): + for given, expect in (("5s", "5s"), (5, "5s"), ("999", "10s"), ("3600s", "10s"), ("0", "1s"), (0, "1s")): cap={} with mock.patch.object(mm,"http_json",side_effect=lambda m,u,headers=None,body=None,timeout=None:(cap.update(url=u) or (200,{"status":"success","data":{"resultType":"vector","result":[]}}))): mm.lambda_handler({"tool_name":"mimir_query","arguments":{"query":"up","timeout":given}},None) @@ -33,7 +33,7 @@ def test_execution_timeout_is_passed_and_clamped(self): cap={} with mock.patch.object(mm,"http_json",side_effect=lambda m,u,headers=None,body=None,timeout=None:(cap.update(url=u) or (200,{"status":"success","data":{"resultType":"vector","result":[]}}))): mm.lambda_handler({"tool_name":"mimir_query","arguments":{"query":"up"}},None) - self.assertNotIn("timeout", _qs(cap["url"])) # unchanged for callers that ask for no bound + self.assertEqual(_qs(cap["url"])["timeout"], ["10s"]) def test_range_seconds_window(self): cap={} diff --git a/agent/lambda/test_prometheus_mcp.py b/agent/lambda/test_prometheus_mcp.py index e8d5abda0..c59cab020 100644 --- a/agent/lambda/test_prometheus_mcp.py +++ b/agent/lambda/test_prometheus_mcp.py @@ -49,7 +49,7 @@ def fake(method, url, headers=None, body=None, timeout=None): cap.update(url=url) return 200, {"status": "success", "data": {"resultType": "vector", "result": []}} - for given, expect in (("5s", "5s"), (5, "5s"), ("999", "60s"), ("3600s", "60s"), ("0", "1s"), (0, "1s")): + for given, expect in (("5s", "5s"), (5, "5s"), ("999", "10s"), ("3600s", "10s"), ("0", "1s"), (0, "1s")): cap = {} with mock.patch.object(pm, "http_json", side_effect=fake): pm.lambda_handler({"tool_name": "prometheus_query", @@ -58,7 +58,7 @@ def fake(method, url, headers=None, body=None, timeout=None): cap = {} with mock.patch.object(pm, "http_json", side_effect=fake): pm.lambda_handler({"tool_name": "prometheus_query", "arguments": {"query": "up"}}, None) - self.assertNotIn("timeout", _qs(cap["url"])) # unchanged for callers that ask for no bound + self.assertEqual(_qs(cap["url"])["timeout"], ["10s"]) def test_range_defaults(self): cap = {} diff --git a/agent/tests/test_health_tools.py b/agent/tests/test_health_tools.py index 84543f990..078105f85 100644 --- a/agent/tests/test_health_tools.py +++ b/agent/tests/test_health_tools.py @@ -21,44 +21,51 @@ class TestHealthTools(unittest.TestCase): def tearDown(self): dh.set_request_conn(None) - def test_prometheus_health_uses_inline_conn_and_probes_healthy(self): + def test_prometheus_health_uses_inline_conn_and_verifies_query(self): with mock.patch.object(prometheus_mcp, "assert_host_allowed"), \ - mock.patch.object(dh, "assert_host_allowed"), \ - mock.patch.object(dh, "http_json", return_value=(200, {})) as hj: + mock.patch.object(prometheus_mcp, "http_json", return_value=(200, { + "status": "success", "data": {"resultType": "vector", "result": [{"metric": {}, "value": [1, "1"]}]}, + })) as hj: resp = prometheus_mcp.lambda_handler( {"tool_name": "prometheus_health", "arguments": {}, "conn_config": {"endpoint": "http://p:9090", "authType": "none"}}, None, ) self.assertEqual(resp["statusCode"], 200) self.assertTrue(json.loads(resp["body"])["ok"]) - self.assertTrue(hj.call_args[0][1].endswith("/-/healthy")) # GET /-/healthy + self.assertIn("/api/v1/query?", hj.call_args[0][1]) + self.assertIn("timeout=1s", hj.call_args[0][1]) - def test_clickhouse_health_probes_ping(self): - with mock.patch.object(dh, "assert_host_allowed"), \ - mock.patch.object(dh, "http_json", return_value=(200, {})) as hj: + def test_clickhouse_health_verifies_readonly_sql(self): + with mock.patch.object(clickhouse_mcp, "assert_host_allowed"), \ + mock.patch.object(clickhouse_mcp, "http_json", return_value=(200, { + "meta": [{"name": "probe", "type": "UInt8"}], "data": [{"probe": 1}], "rows": 1, + })) as hj: resp = clickhouse_mcp.lambda_handler( {"tool_name": "clickhouse_health", "arguments": {}, "conn_config": {"endpoint": "http://ch:8123", "authType": "basic", "username": "u", "password": "p"}}, None, ) self.assertEqual(resp["statusCode"], 200) self.assertTrue(json.loads(resp["body"])["ok"]) - self.assertTrue(hj.call_args[0][1].endswith("/ping")) + self.assertIn("readonly=1", hj.call_args[0][1]) + self.assertIn("max_execution_time=1", hj.call_args[0][1]) - def test_loki_health_probes_ready(self): - with mock.patch.object(dh, "assert_host_allowed"), \ - mock.patch.object(dh, "http_json", return_value=(200, {})) as hj: + def test_loki_health_verifies_tenant_query(self): + with mock.patch.object(loki_mcp, "assert_host_allowed"), \ + mock.patch.object(loki_mcp, "http_json", return_value=(200, { + "status": "success", "data": {"resultType": "streams", "result": []}, + })) as hj: resp = loki_mcp.lambda_handler( {"tool_name": "loki_health", "arguments": {}, "conn_config": {"endpoint": "http://loki:3100", "authType": "none", "org_id": "t1"}}, None, ) self.assertEqual(resp["statusCode"], 200) self.assertTrue(json.loads(resp["body"])["ok"]) - self.assertTrue(hj.call_args[0][1].endswith("/ready")) + self.assertIn("/loki/api/v1/query_range?", hj.call_args[0][1]) + self.assertEqual(hj.call_args.kwargs["headers"]["X-Scope-OrgID"], "t1") def test_health_reports_failure_on_http_error(self): with mock.patch.object(prometheus_mcp, "assert_host_allowed"), \ - mock.patch.object(dh, "assert_host_allowed"), \ - mock.patch.object(dh, "http_json", return_value=(502, {})): + mock.patch.object(prometheus_mcp, "http_json", return_value=(502, {})): resp = prometheus_mcp.lambda_handler( {"tool_name": "prometheus_health", "conn_config": {"endpoint": "http://p:9090"}}, None, ) diff --git a/terraform/foundation/ai.tf b/terraform/foundation/ai.tf index 1908b8537..d3f94176d 100644 --- a/terraform/foundation/ai.tf +++ b/terraform/foundation/ai.tf @@ -58,6 +58,28 @@ variable "mimir_vpc_enabled" { default = false } +variable "jaeger_vpc_enabled" { + type = bool + default = false + nullable = false + description = "Attach the Jaeger query Lambda to private subnets. Requires agentcore_enabled and integrations_enabled; routing, DNS and target ingress must be operator-configured." + validation { + condition = !var.jaeger_vpc_enabled || (var.agentcore_enabled && var.integrations_enabled) + error_message = "jaeger_vpc_enabled requires agentcore_enabled and integrations_enabled." + } +} + +variable "dynatrace_vpc_enabled" { + type = bool + default = false + nullable = false + description = "Attach the Dynatrace environment API Lambda to private subnets for a private ActiveGate endpoint. Requires agentcore_enabled and integrations_enabled. Does not affect the official hosted MCP preset." + validation { + condition = !var.dynatrace_vpc_enabled || (var.agentcore_enabled && var.integrations_enabled) + error_message = "dynatrace_vpc_enabled requires agentcore_enabled and integrations_enabled." + } +} + variable "istio_vpc_enabled" { type = bool description = "Attach the istio-read Lambda to the private subnets so it can reach a PRIVATE-ONLY EKS API endpoint. Requires agentcore_enabled. Default false → no-op ($0); off = non-VPC (reaches a public/public+private cluster endpoint). PERSIST in live terraform.tfvars." @@ -509,10 +531,10 @@ resource "aws_iam_role_policy" "agent_lambda_opensearch" { }) } -# ENI perms for the opensearch-mcp Lambda ONLY when it is VPC-attached (opensearch_vpc_enabled). +# ENI permissions exist only while a managed Lambda VPC option is enabled. # Compound-gated: references agent_lambda[0], which exists only when agentcore_enabled → guard both. resource "aws_iam_role_policy" "agent_lambda_vpc_eni" { - count = var.agentcore_enabled && (var.opensearch_vpc_enabled || var.clickhouse_vpc_enabled || var.prometheus_vpc_enabled || var.loki_vpc_enabled || var.tempo_vpc_enabled || var.mimir_vpc_enabled || var.istio_vpc_enabled) ? 1 : 0 + count = var.agentcore_enabled && (var.opensearch_vpc_enabled || var.clickhouse_vpc_enabled || var.prometheus_vpc_enabled || var.loki_vpc_enabled || var.tempo_vpc_enabled || var.mimir_vpc_enabled || var.jaeger_vpc_enabled || var.dynatrace_vpc_enabled || var.istio_vpc_enabled) ? 1 : 0 name = "${var.project}-agent-lambda-vpc-eni" role = aws_iam_role.agent_lambda[0].id policy = jsonencode({ @@ -967,10 +989,10 @@ resource "aws_lambda_function" "agent" { } # Per-Lambda VPC opt-in: attach a connector to the private subnets ONLY when its _vpc_enabled - # flag is set (opensearch/clickhouse/prometheus/loki/tempo/mimir for VPC-only datasources; + # flag is set (opensearch/clickhouse/prometheus/loki/tempo/mimir/jaeger/dynatrace for private datasources; # istio-read for a private-only EKS API endpoint). Off (default) → no vpc_config → non-VPC. dynamic "vpc_config" { - for_each = ((each.key == "opensearch-mcp" && var.opensearch_vpc_enabled) || (each.key == "clickhouse-mcp" && var.clickhouse_vpc_enabled) || (each.key == "prometheus-mcp" && var.prometheus_vpc_enabled) || (each.key == "loki-mcp" && var.loki_vpc_enabled) || (each.key == "tempo-mcp" && var.tempo_vpc_enabled) || (each.key == "mimir-mcp" && var.mimir_vpc_enabled) || (each.key == "istio-read" && var.istio_vpc_enabled)) ? [1] : [] + for_each = ((each.key == "opensearch-mcp" && var.opensearch_vpc_enabled) || (each.key == "clickhouse-mcp" && var.clickhouse_vpc_enabled) || (each.key == "prometheus-mcp" && var.prometheus_vpc_enabled) || (each.key == "loki-mcp" && var.loki_vpc_enabled) || (each.key == "tempo-mcp" && var.tempo_vpc_enabled) || (each.key == "mimir-mcp" && var.mimir_vpc_enabled) || (each.key == "jaeger-mcp" && var.jaeger_vpc_enabled) || (each.key == "dynatrace-mcp" && var.dynatrace_vpc_enabled) || (each.key == "istio-read" && var.istio_vpc_enabled)) ? [1] : [] content { subnet_ids = local.private_subnet_ids security_group_ids = [aws_security_group.service.id] diff --git a/terraform/foundation/tests/runtime_iam.tftest.hcl b/terraform/foundation/tests/runtime_iam.tftest.hcl index 89d6afd56..13e18afaf 100644 --- a/terraform/foundation/tests/runtime_iam.tftest.hcl +++ b/terraform/foundation/tests/runtime_iam.tftest.hcl @@ -80,6 +80,40 @@ run "defaults_remain_dark" { condition = (length(aws_iam_role_policy.agentcore) == 0 && length(aws_iam_role_policy.official_mcp_credentials) == 0 && length(aws_iam_role_policy.steampipe_task) == 0 && length(aws_iam_role_policy.worker_lambda) == 0 && !var.inventory_host_only && var.steampipe_image_digest == null && var.worker_image_digest == null && !var.remediation_enabled && !var.diagnosis_notify_enabled && !var.integrations_write_enabled) error_message = "Core runtime and host/image overrides must remain opt-in." } + assert { + condition = !var.jaeger_vpc_enabled && !var.dynatrace_vpc_enabled && length(aws_iam_role_policy.agent_lambda_vpc_eni) == 0 + error_message = "Private external datasource networking must remain default-off." + } +} + +run "private_datasource_flags_require_applied_prerequisites" { + command = plan + variables { + jaeger_vpc_enabled = true + dynatrace_vpc_enabled = true + } + expect_failures = [var.jaeger_vpc_enabled, var.dynatrace_vpc_enabled] +} + +run "private_jaeger_and_dynatrace_attach_only_the_selected_lambdas" { + command = plan + variables { + agentcore_enabled = true + integrations_enabled = true + jaeger_vpc_enabled = true + dynatrace_vpc_enabled = true + } + assert { + condition = ( + aws_lambda_function.agent["jaeger-mcp"].vpc_config[0].subnet_ids == toset(var.existing_private_subnet_ids) && + aws_lambda_function.agent["dynatrace-mcp"].vpc_config[0].subnet_ids == toset(var.existing_private_subnet_ids) && + length(aws_lambda_function.agent["datadog-mcp"].vpc_config) == 0 && + length(aws_lambda_function.agent["prometheus-mcp"].vpc_config) == 0 && + length(aws_iam_role_policy.agent_lambda_vpc_eni) == 1 && + !var.official_mcp_enabled && !var.remediation_enabled && !var.integrations_write_enabled + ) + error_message = "Private datasource attachment must be selective and must not activate MCP/write gates." + } } run "explicit_inventory_targets_pin_metadata_permissions_and_actual_principal" { From 434f5f66b872ce2d3e27ebc93072dc1902f675e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=98=A4=EC=A4=80=EC=84=9D=28Junseok=20Oh=29DevOps?= <52226147+Atom-oh@users.noreply.github.com> Date: Sun, 18 Oct 2026 13:58:50 +0000 Subject: [PATCH 02/27] fix(datasources): share one request deadline and disclose limit saturation Review follow-ups for the connector bounds: connect, headers and body share one monotonic deadline so a slow upstream cannot double the aligned HTTP timeout past the Lambda wall; oversized or slow error bodies keep their HTTP status. Loki and Jaeger report limit saturation as partial with requested/returned counts, separately from byte-bound loss. Connection probes return latency_ms, accept empty or warned results, and keep the SSRF classification. The default kind mirror is stamped with its instance id, the id-keyed lookup accepts only that default's mirror (legacy unstamped mirrors keep working with a warning until the next save), and timeoutS rides the console conn config for every kind. Restores the resolve-and-recheck disclosure that the stacked agent change replaces. Co-Authored-By: Claude Fable 5.1 --- agent/lambda/clickhouse_mcp.py | 12 +-- agent/lambda/datadog_mcp.py | 16 ++-- agent/lambda/datasource_http.py | 64 ++++++++++++-- agent/lambda/dynatrace_mcp.py | 18 ++-- agent/lambda/jaeger_mcp.py | 26 +++--- agent/lambda/loki_mcp.py | 42 +++++---- agent/lambda/mimir_mcp.py | 38 +++------ agent/lambda/prometheus_mcp.py | 28 ++---- agent/lambda/tempo_mcp.py | 16 ++-- agent/lambda/test_datadog_mcp.py | 6 +- agent/lambda/test_datasource_http.py | 6 +- agent/lambda/test_external_connections.py | 89 ++++++++++++++++++-- web/app/api/datasources/manage/route.test.ts | 4 +- web/app/api/datasources/manage/route.ts | 4 +- web/lib/datasources.test.ts | 8 +- web/lib/datasources.ts | 17 ++-- web/lib/integration-credentials.id.test.ts | 7 ++ web/lib/integration-credentials.ts | 8 +- 18 files changed, 256 insertions(+), 153 deletions(-) diff --git a/agent/lambda/clickhouse_mcp.py b/agent/lambda/clickhouse_mcp.py index 38d08f56f..7ca790ebf 100644 --- a/agent/lambda/clickhouse_mcp.py +++ b/agent/lambda/clickhouse_mcp.py @@ -21,9 +21,9 @@ SsrfBlocked, assert_host_allowed, auth_headers, - health, http_json, load_datasource, + query_probe, set_request_conn, ) from sql_readonly_guard import assert_read_only as _shared_assert_read_only @@ -274,14 +274,10 @@ def clickhouse_schema(args): def clickhouse_health(args): """Verify read-only SQL access using one constant row and a one-second ceiling.""" - try: + def run(): result = _run_sql("SELECT 1 AS probe", 1, max_execution_time=1) - payload = json.loads(result["body"]) - verified = result["statusCode"] == 200 and payload.get("collectionStatus") in ("ok", "partial") - return ok({"ok": verified, "verified": ["query"] if verified else [], - **({} if verified else {"error": "ClickHouse query access could not be verified"})}) - except Exception: - return ok({"ok": False, "error": "ClickHouse query access could not be verified"}) + return json.loads(result["body"]).get("collectionStatus") if result["statusCode"] == 200 else None + return ok(query_probe(run, "ClickHouse query access could not be verified", accepted=("ok", "partial"))) _TOOLS = { diff --git a/agent/lambda/datadog_mcp.py b/agent/lambda/datadog_mcp.py index faaa8a189..94c4ed3de 100644 --- a/agent/lambda/datadog_mcp.py +++ b/agent/lambda/datadog_mcp.py @@ -16,7 +16,7 @@ from cross_account import resolve_tool_name from datasource_http import ( NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, http_json, load_datasource, - set_request_conn, request_timeout, finite_number, observed_payload, + query_probe, set_request_conn, request_timeout, finite_number, observed_payload, ) SLUG = "datadog" @@ -122,15 +122,15 @@ def datadog_schema(args): def datadog_health(args): """Validate configured API keys, then metric-query access (key pair or bearer).""" - creds = load_datasource(SLUG) - assert_host_allowed(creds["endpoint"]) - try: + def run(): + creds = load_datasource(SLUG) + assert_host_allowed(creds["endpoint"]) # The API-key validator does not validate bearer access tokens. Those use # the supported metric-query operation itself as the authentication proof. if any(name.lower() == "dd-api-key" for name in _headers(creds)): v = _get(creds, "/api/v1/validate") if not isinstance(v, dict) or v.get("valid") is not True: - return ok({"ok": False, "error": "Datadog API key validation failed"}) + return None end = int(time.time()) # A one-minute, single-metric query with a probe-only host filter verifies # metric-query permission without fetching the fleet's real metric series. @@ -138,10 +138,8 @@ def datadog_health(args): "query": "avg:system.cpu.user{host:awsops-connection-probe}", "from": str(end - 60), "to": str(end), }) - verified = isinstance(probe, dict) and probe.get("status") == "ok" and isinstance(probe.get("series"), list) - return ok({"ok": verified, **({} if verified else {"error": "Datadog metric-query access could not be verified"})}) - except _ApiError: - return ok({"ok": False, "error": "Datadog credentials or metric-query access could not be verified"}) + return "ok" if isinstance(probe, dict) and probe.get("status") == "ok" and isinstance(probe.get("series"), list) else None + return ok(query_probe(run, "Datadog credentials or metric-query access could not be verified")) _TOOLS = { diff --git a/agent/lambda/datasource_http.py b/agent/lambda/datasource_http.py index f59e7c17b..dd0684b90 100644 --- a/agent/lambda/datasource_http.py +++ b/agent/lambda/datasource_http.py @@ -24,8 +24,9 @@ calls assert_host_allowed(endpoint) immediately before http_json(same endpoint) — that invariant is what makes the cache safe; it is reset every invocation by set_request_conn() (called at the top of every lambda_handler) so a warm container never reuses a stale pinned IP across invocations. -The separate custom-MCP transport pins its request-local validated address set through -agent/pinned_http.py; the two transports retain their different private-network contracts. +(A structurally identical resolve-and-recheck gap exists in agent/agent.py's `_assert_host_allowed` +for the separate egress-integrations transport — untouched here; that's a different call path not +exercised by the pentest's datasource/schema-fetch findings, and needs its own fix.) """ import base64 import functools @@ -220,6 +221,24 @@ def set_request_conn(conn): _PINNED_IP_CACHE = {} +def query_probe(run, error, accepted=("ok", "empty", "partial")): + """Time one real bounded query for the connection test. `run` executes the connector's own + query tool and returns its `collectionStatus` (None when the response was unrecognizable). + Returns {ok, verified, latency_ms, error?} — fixed sentences only, never upstream text; the + connector's own classification (SSRF policy vs. unverifiable) is kept because an admin needs it.""" + t0 = time.monotonic() + try: + state = run() + except SsrfBlocked: + state, error = None, "endpoint rejected by the private-network policy" + except Exception: + state = None + latency = int((time.monotonic() - t0) * 1000) + verified = state in accepted + return {"ok": verified, "verified": ["query"] if verified else [], "latency_ms": latency, + **({} if verified else {"error": error})} + + def health(creds, path): """Lightweight connectivity probe: GET endpoint+path with auth, SSRF-guarded. {ok, latency_ms, error?}.""" import time as _t @@ -272,7 +291,12 @@ def load_datasource(slug, instance_id=None): 1. the request-scoped inline conn-config (`_REQUEST_CONN`) — the trusted BFF path only; 2. the per-instance secret key `map[str(instance_id)]` — the credential-blind worker path, where the caller sends ONLY an instance_id and the connector reads the secret server-side; - 3. the `map[slug]` kind-mirror, only when no explicit instance was requested. + 3. the `map[slug]` kind-mirror: when no explicit instance was requested, or when the + requested id IS the default instance the mirror was written for (`instanceId` stamp). + An explicit id never inherits a mirror stamped for a different instance. A mirror without a + stamp predates the stamp (a migrated default whose id key was never written); it is accepted + for an explicit id with a warning so that default keeps working until its next save writes the + id key and the stamp. `instance_id` is a pure argument (no module-global), so warm-container reuse cannot bleed one invocation's instance into the next.""" if _REQUEST_CONN is not None: @@ -281,7 +305,16 @@ def load_datasource(slug, instance_id=None): creds = m.get(str(instance_id)) if instance_id is not None else None if instance_id is not None: if not isinstance(creds, dict) or not creds.get("endpoint"): - raise NotConnected("requested datasource instance is not configured") + mirror = m.get(slug) + stamp = mirror.get("instanceId") if isinstance(mirror, dict) else None + if isinstance(mirror, dict) and mirror.get("endpoint") and ( + stamp is None or str(stamp) == str(instance_id)): + if stamp is None: + print(f"[datasource_http] WARNING: {slug} instance {instance_id} resolved via an " + "unstamped kind mirror; re-save the default instance to write its id key") + creds = mirror + else: + raise NotConnected("requested datasource instance is not configured") else: creds = m.get(slug) if not isinstance(creds, dict) or not creds.get("endpoint"): @@ -386,11 +419,12 @@ def _parse(raw): return {"raw": raw.decode("utf-8", "replace")[:4000]} -def _read_bounded(response, max_bytes, timeout): - """Read at most budget+1 bytes; never trust Content-Length or parse before the bound.""" +def _read_bounded(response, max_bytes, deadline): + """Read at most budget+1 bytes before the absolute monotonic `deadline`; never trust + Content-Length or parse before the bound. The deadline is the request's — connect, headers + and body share one budget, so a slow-but-alive upstream cannot double it.""" chunks = [] total = 0 - deadline = time.monotonic() + timeout read = getattr(response, "read1", response.read) try: while True: @@ -413,9 +447,21 @@ def http_json(method, url, headers=None, body=None, timeout=HTTP_TIMEOUT, max_by in order with fallback) — see module docstring (IP-pinning).""" data = body if isinstance(body, (bytes, bytearray)) else (body.encode() if isinstance(body, str) else None) req = urllib.request.Request(url, data=data, method=method, headers=headers or {}) + # One wall-clock budget per request: the socket timeout bounds connect + headers, and the body + # read gets only what is left, so total time stays <= `timeout` (the connector Lambda's 60s wall + # assumes the aligned HTTP timeout, at most 55s + 3s, is the whole request). + deadline = time.monotonic() + timeout try: resp = _opener_for(urlparse(url).hostname).open(req, timeout=timeout) - return resp.getcode(), _parse(_read_bounded(resp, max_bytes, timeout)) + return resp.getcode(), _parse(_read_bounded(resp, max_bytes, deadline)) except urllib.error.HTTPError as e: - raw = _read_bounded(e, min(max_bytes, MAX_ERROR_BYTES), timeout) + # Keep the (status, body) contract for error responses: an oversized or slow error body is + # reported as such under its real HTTP status instead of replacing the status with a + # transport exception. + try: + raw = _read_bounded(e, min(max_bytes, MAX_ERROR_BYTES), deadline) + except ResponseTooLarge: + return e.code, {"error": f"error body exceeded {MAX_ERROR_BYTES} bytes"} + except TimeoutError: + return e.code, {"error": "error body read deadline exceeded"} return e.code, _parse(raw) diff --git a/agent/lambda/dynatrace_mcp.py b/agent/lambda/dynatrace_mcp.py index 0329530ed..a73b6146e 100644 --- a/agent/lambda/dynatrace_mcp.py +++ b/agent/lambda/dynatrace_mcp.py @@ -16,7 +16,7 @@ from cross_account import resolve_tool_name from datasource_http import ( NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, http_json, load_datasource, - set_request_conn, request_timeout, finite_number, observed_payload, + query_probe, set_request_conn, request_timeout, finite_number, observed_payload, ) SLUG = "dynatrace" @@ -149,18 +149,14 @@ def dynatrace_schema(args): def dynatrace_health(args): """Verify Metrics v2 query access with a one-minute, one-resolution query.""" - creds = load_datasource(SLUG) - assert_host_allowed(creds["endpoint"]) - try: - result = json.loads(dynatrace_query({ + def run(): + creds = load_datasource(SLUG) + assert_host_allowed(creds["endpoint"]) + return json.loads(dynatrace_query({ "query": "builtin:host.cpu.usage:avg", "start": "now-1m", "end": "now", "resolution": "1m", - })["body"]) - verified = result.get("collectionStatus") in ("ok", "empty", "partial") - return ok({"ok": verified, "verified": ["query"] if verified else [], - **({} if verified else {"error": "Dynatrace metric-query access could not be verified"})}) - except Exception: - return ok({"ok": False, "error": "Dynatrace metric-query access could not be verified"}) + })["body"]).get("collectionStatus") + return ok(query_probe(run, "Dynatrace metric-query access could not be verified")) _TOOLS = { diff --git a/agent/lambda/jaeger_mcp.py b/agent/lambda/jaeger_mcp.py index d9b1187c7..92eb38fac 100644 --- a/agent/lambda/jaeger_mcp.py +++ b/agent/lambda/jaeger_mcp.py @@ -17,8 +17,8 @@ from cross_account import resolve_tool_name from datasource_http import ( - NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, health, http_json, load_datasource, - set_request_conn, request_timeout, observed_payload, + NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, http_json, load_datasource, + query_probe, set_request_conn, request_timeout, observed_payload, ) SLUG = "jaeger" @@ -130,11 +130,15 @@ def jaeger_search(args): "durationMs": round(dur_us / 1000, 1), "startTime": root.get("startTime", 0), }) - truncated = len(traces) >= requested + # Reaching the requested limit is saturation, not loss: Jaeger returns no marker, so more traces + # may exist and the result is `partial`, but only a real drop (byte bound) sets `truncated`. + saturated = len(traces) >= requested http_status = getattr(data, "http_status", 200) - state = "unknown" if invalid or http_status not in (200, 206) else "partial" if truncated or http_status == 206 else "ok" if rows else "empty" + state = "unknown" if invalid or http_status not in (200, 206) else "partial" if saturated or http_status == 206 else "ok" if rows else "empty" payload, btr = _byte_bound({"traces": rows}) - return ok({**payload, "truncated": truncated or btr, + if btr: + payload.pop("preview", None) # never echo provider content, even as a bounded preview + return ok({**payload, "truncated": btr, "limitSaturated": saturated, "collectionStatus": "unknown" if invalid else "partial" if btr else state, "requestedLimit": requested, "returnedCount": len(rows)}) @@ -173,17 +177,13 @@ def jaeger_schema(args): def jaeger_health(args): """Verify a recognizable query response, not an arbitrary HTTP 200.""" - try: + def run(): end = int(time.time()) - result = json.loads(jaeger_search({ + return json.loads(jaeger_search({ "query": "service=awsops-connection-probe&limit=1", "start": str(end - 60), "end": str(end), - })["body"]) - verified = result.get("collectionStatus") in ("ok", "empty", "partial") - return ok({"ok": verified, "verified": ["query"] if verified else [], - **({} if verified else {"error": "Jaeger query access could not be verified"})}) - except Exception: - return ok({"ok": False, "error": "Jaeger query access could not be verified"}) + })["body"]).get("collectionStatus") + return ok(query_probe(run, "Jaeger query access could not be verified")) _TOOLS = { diff --git a/agent/lambda/loki_mcp.py b/agent/lambda/loki_mcp.py index 7f9693c32..18ffa855a 100644 --- a/agent/lambda/loki_mcp.py +++ b/agent/lambda/loki_mcp.py @@ -15,8 +15,8 @@ from cross_account import resolve_tool_name from datasource_http import ( - NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, health, http_json, load_datasource, - set_request_conn, request_timeout, observed_payload, finite_number, + NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, http_json, load_datasource, + query_probe, set_request_conn, request_timeout, observed_payload, finite_number, ) SLUG = "loki" @@ -109,7 +109,10 @@ def _valid_row(row, kind): return True -def _query_response(data): +def _query_response(data, requested=None): + """Normalize a Loki query envelope. `requested` is the caller's `limit`: Loki returns at most + that many log lines with no marker, so reaching it is disclosed as `limitSaturated` and the + result is `partial` — more lines may exist (mirrors Jaeger's requested/returned contract).""" valid = (isinstance(data, dict) and data.get("resultType") in ("streams", "matrix", "vector") and isinstance(data.get("result"), list) and not getattr(data, "source_unverified", False)) kind = data.get("resultType") if isinstance(data, dict) else None @@ -120,12 +123,15 @@ def _query_response(data): valid = valid and len(rows) == len(retained) bounded, truncated = _bound({"resultType": kind, "result": rows}) truncated = truncated or len(source_rows) > MAX_STREAMS + returned = sum(len(row.get("values") or []) for row in rows if isinstance(row, dict)) if kind == "streams" else len(rows) + saturated = bool(requested) and returned >= requested status = getattr(data, "http_status", 200) state = ("unknown" if not valid or status not in (200, 206) else - "partial" if truncated or status == 206 or getattr(data, "source_partial", False) else + "partial" if truncated or saturated or status == 206 or getattr(data, "source_partial", False) else "ok" if data["result"] else "empty") return ok({"truncated": truncated, **(bounded if isinstance(bounded, dict) else {"result": None}), - "collectionStatus": state}) + "collectionStatus": state, + **({"requestedLimit": requested, "returnedCount": returned, "limitSaturated": saturated} if requested else {})}) def _bound(data): @@ -172,7 +178,7 @@ def loki_query_range(args): if args.get("step"): params["step"] = str(args["step"]) data = _get(_ds(), "/loki/api/v1/query_range", params) - return _query_response(data) + return _query_response(data, _requested(params["limit"])) def loki_query(args): @@ -183,7 +189,12 @@ def loki_query(args): if args.get("time"): params["time"] = _parse_time_ns(args.get("time")) data = _get(_ds(), "/loki/api/v1/query", params) - return _query_response(data) + return _query_response(data, _requested(params["limit"])) + + +def _requested(limit): + """The positive integer limit actually sent to Loki, or None when it was not a usable number.""" + return int(limit) if re.fullmatch(r"[1-9]\d{0,8}", str(limit)) else None def loki_labels(args): @@ -227,22 +238,17 @@ def loki_schema(args): def loki_health(args): - """Verify the tenant-scoped query endpoint with a one-minute, one-line search.""" - try: + """Verify the tenant-scoped query endpoint with a one-minute, one-line search, judged by the + same `_query_response` verdict the query tools use.""" + def run(): end = int(time.time()) - result = _get(_ds(), "/loki/api/v1/query_range", { + data = _get(_ds(), "/loki/api/v1/query_range", { "query": '{service_name="awsops-connection-probe"}', "start": str((end - 60) * 1_000_000_000), "end": str(end * 1_000_000_000), "limit": "1", }) - verified = (isinstance(result, dict) and result.get("resultType") == "streams" - and isinstance(result.get("result"), list) - and all(isinstance(row, dict) and isinstance(row.get("values"), list) - for row in result["result"])) - return ok({"ok": verified, "verified": ["query"] if verified else [], - **({} if verified else {"error": "Loki tenant query access could not be verified"})}) - except Exception: - return ok({"ok": False, "error": "Loki tenant query access could not be verified"}) + return json.loads(_query_response(data, 1)["body"]).get("collectionStatus") + return ok(query_probe(run, "Loki tenant query access could not be verified")) _TOOLS["loki_health"] = loki_health diff --git a/agent/lambda/mimir_mcp.py b/agent/lambda/mimir_mcp.py index 422a3537f..4e381819e 100644 --- a/agent/lambda/mimir_mcp.py +++ b/agent/lambda/mimir_mcp.py @@ -9,12 +9,12 @@ import math import re import time -from urllib.parse import urlencode +from urllib.parse import urlencode, urlparse from cross_account import resolve_tool_name from datasource_http import ( - NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, health, http_json, load_datasource, - set_request_conn, + NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, http_json, load_datasource, + query_probe, set_request_conn, execution_timeout, request_timeout, ) @@ -63,11 +63,12 @@ def _ds(): def _get(creds, path, params, http_timeout=None, *, with_status=False): if path.startswith(BASE): - prefix = creds.get("apiPrefix", "/prometheus") + prefix = creds.get("apiPrefix") + if prefix is None: # absent or stored null = default mount; an explicit "" = API at the root + prefix = "/prometheus" if not isinstance(prefix, str) or len(prefix) > 128 or not re.fullmatch(r"(?:/[A-Za-z0-9_-]+)*", prefix): raise _ApiError("invalid Mimir API prefix") # An API base already ending in the configured prefix must not duplicate it. - from urllib.parse import urlparse endpoint_path = urlparse(creds["endpoint"]).path.rstrip("/") if prefix and endpoint_path.endswith(prefix): prefix = "" @@ -196,20 +197,6 @@ def _query_result(observed, *, allow_scalar=False): -def _timeout_param(v): - """Optional Prometheus-API `timeout` (e.g. "5s"), clamped to 1..60s. Same knob prometheus_mcp.py grew: - the connector bounds response SIZE but had no execution bound, which matters now that a caller can run - model-written PromQL through it (review — the diag-signal dry run passes 5s for mimir too).""" - # No digit cap: `\d{1,3}` made "1000" or "3600s" fall through to None, i.e. NO bound — a clamp that - # fails open for exactly the values that need clamping (review MINOR). Only non-numeric input is None. - # `v or ""` turns numeric 0 into "" (falsy), which then fails the match and returns None — - # i.e. NO bound for exactly the value that most needs clamping. Only skip on a true absence. - if v is None or v == "": - return None - m = re.fullmatch(r"\s*(\d+)s?\s*", str(v)) - return f"{max(1, min(60, int(m.group(1))))}s" if m else None - - def mimir_query(args): query = (args.get("query") or "").strip() if not query: @@ -357,14 +344,11 @@ def mimir_metric_meta(args): def mimir_health(args): - """Verify tenant-scoped query access, independently of process readiness.""" - try: - result = json.loads(mimir_query({"query": "vector(1)", "timeout": "1s"})["body"]) - verified = result.get("collectionStatus") == "ok" - return ok({"ok": verified, "verified": ["query"] if verified else [], - **({} if verified else {"error": "Mimir tenant query access could not be verified"})}) - except Exception: - return ok({"ok": False, "error": "Mimir tenant query access could not be verified"}) + """Verify tenant-scoped query access, independently of process readiness. An empty or + upstream-warned (`partial`) result still proves access.""" + return ok(query_probe( + lambda: json.loads(mimir_query({"query": "vector(1)", "timeout": "1s"})["body"]).get("collectionStatus"), + "Mimir tenant query access could not be verified")) _TOOLS["mimir_health"] = mimir_health diff --git a/agent/lambda/prometheus_mcp.py b/agent/lambda/prometheus_mcp.py index 62011a10a..2728374a9 100644 --- a/agent/lambda/prometheus_mcp.py +++ b/agent/lambda/prometheus_mcp.py @@ -21,9 +21,9 @@ SsrfBlocked, assert_host_allowed, auth_headers, - health, http_json, load_datasource, + query_probe, set_request_conn, execution_timeout, request_timeout, @@ -196,19 +196,6 @@ def _query_result(observed, *, allow_scalar=False): -def _timeout_param(v): - """Optional Prometheus API `timeout` (e.g. "5s"), clamped to 1..60s. The connector bounds response SIZE - but had no execution bound, which matters now that a caller can run model-written PromQL (review).""" - # No digit cap: `\d{1,3}` made "1000" or "3600s" fall through to None, i.e. NO bound — a clamp that - # fails open for exactly the values that need clamping (review MINOR). Only non-numeric input is None. - # `v or ""` turns numeric 0 into "" (falsy), which then fails the match and returns None — - # i.e. NO bound for exactly the value that most needs clamping. Only skip on a true absence. - if v is None or v == "": - return None - m = re.fullmatch(r"\s*(\d+)s?\s*", str(v)) - return f"{max(1, min(60, int(m.group(1))))}s" if m else None - - def prometheus_query(args): query = (args.get("query") or "").strip() if not query: @@ -299,14 +286,11 @@ def prometheus_schema(args): def prometheus_health(args): - """Verify the actual authenticated query API using a bounded scalar query.""" - try: - result = json.loads(prometheus_query({"query": "vector(1)", "timeout": "1s"})["body"]) - verified = result.get("collectionStatus") == "ok" - return ok({"ok": verified, "verified": ["query"] if verified else [], - **({} if verified else {"error": "Prometheus query access could not be verified"})}) - except Exception: - return ok({"ok": False, "error": "Prometheus query access could not be verified"}) + """Verify the actual authenticated query API using a bounded scalar query. A successful query + whose result is empty or carries an upstream warning (`partial`) still proves access.""" + return ok(query_probe( + lambda: json.loads(prometheus_query({"query": "vector(1)", "timeout": "1s"})["body"]).get("collectionStatus"), + "Prometheus query access could not be verified")) def prometheus_metric_meta(args): diff --git a/agent/lambda/tempo_mcp.py b/agent/lambda/tempo_mcp.py index 3aabed856..0546ac4fd 100644 --- a/agent/lambda/tempo_mcp.py +++ b/agent/lambda/tempo_mcp.py @@ -18,8 +18,8 @@ from cross_account import resolve_tool_name from datasource_http import ( - NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, health, http_json, load_datasource, - set_request_conn, request_timeout, + NotConnected, SsrfBlocked, assert_host_allowed, auth_headers, http_json, load_datasource, + query_probe, set_request_conn, request_timeout, ) SLUG = "tempo" @@ -543,17 +543,13 @@ def tempo_schema(args): def tempo_health(args): """Verify query access using a one-minute, one-trace search, not process readiness.""" - try: + def run(): end = int(time.time()) - result = json.loads(tempo_search({ + return json.loads(tempo_search({ "query": '{ resource.service.name = "awsops-connection-probe" }', "start": str(end - 60), "end": str(end), "limit": "1", - })["body"]) - verified = result.get("collectionStatus") in ("ok", "empty", "partial") - return ok({"ok": verified, "verified": ["query"] if verified else [], - **({} if verified else {"error": "Tempo tenant query access could not be verified"})}) - except Exception: - return ok({"ok": False, "error": "Tempo tenant query access could not be verified"}) + })["body"]).get("collectionStatus") + return ok(query_probe(run, "Tempo tenant query access could not be verified")) _TOOLS["tempo_health"] = tempo_health diff --git a/agent/lambda/test_datadog_mcp.py b/agent/lambda/test_datadog_mcp.py index 5c9535e2c..75f9c4259 100644 --- a/agent/lambda/test_datadog_mcp.py +++ b/agent/lambda/test_datadog_mcp.py @@ -28,7 +28,8 @@ def test_health_accepts_a_successfully_queried_empty_probe_and_bounds_its_window with patch.object(dd, "load_datasource", return_value=CREDS), patch.object(dd, "assert_host_allowed"), \ patch.object(dd.time, "time", return_value=1000), \ patch.object(dd, "_get", side_effect=[{"valid": True}, {"status": "ok", "series": []}]) as request: - assert body(dd.datadog_health({})) == {"ok": True} + probe = body(dd.datadog_health({})) + assert probe["ok"] is True and probe["verified"] == ["query"] and isinstance(probe["latency_ms"], int) params = request.call_args_list[1].args[2] assert params["from"] == "940" and params["to"] == "1000" assert "{*}" not in params["query"] @@ -51,7 +52,8 @@ def test_bearer_connection_tests_the_metric_operation_without_api_key_validation creds = {"endpoint": CREDS["endpoint"], "authType": "bearer", "token": "synthetic-bearer"} with patch.object(dd, "load_datasource", return_value=creds), patch.object(dd, "assert_host_allowed"), \ patch.object(dd, "_get", return_value={"status": "ok", "series": []}) as request: - assert body(dd.datadog_health({})) == {"ok": True} + probe = body(dd.datadog_health({})) + assert probe["ok"] is True and probe["verified"] == ["query"] and isinstance(probe["latency_ms"], int) request.assert_called_once() assert request.call_args.args[1] == "/api/v1/query" diff --git a/agent/lambda/test_datasource_http.py b/agent/lambda/test_datasource_http.py index 569cc66ab..e09f39e36 100644 --- a/agent/lambda/test_datasource_http.py +++ b/agent/lambda/test_datasource_http.py @@ -85,11 +85,13 @@ def test_not_connected(self): dh.load_datasource("clickhouse") def test_load_datasource_instance_id_pure_arg(self): - # per-instance id key wins; falls back to the kind-mirror when the id key is absent/blank. - m = {"5": {"endpoint": "http://prom-a:9090"}, "prometheus": {"endpoint": "http://prom-default:9090"}} + # per-instance id key wins; the kind mirror serves the no-id path and the stamped default id, + # never a sibling id. + m = {"5": {"endpoint": "http://prom-a:9090"}, "prometheus": {"endpoint": "http://prom-default:9090", "instanceId": "1"}} with mock.patch.object(dh, "_load_secret_map", return_value=m): self.assertEqual(dh.load_datasource("prometheus", instance_id=5)["endpoint"], "http://prom-a:9090") self.assertEqual(dh.load_datasource("prometheus")["endpoint"], "http://prom-default:9090") # kind mirror + self.assertEqual(dh.load_datasource("prometheus", instance_id=1)["endpoint"], "http://prom-default:9090") # stamped default with self.assertRaises(dh.NotConnected): dh.load_datasource("prometheus", instance_id=999) diff --git a/agent/lambda/test_external_connections.py b/agent/lambda/test_external_connections.py index 37ee517d8..026e9397c 100644 --- a/agent/lambda/test_external_connections.py +++ b/agent/lambda/test_external_connections.py @@ -43,18 +43,36 @@ def test_byte_bound_precedes_parse_and_closes_success_stream(size): def test_error_body_is_bounded_and_closed_without_echoing_content(): stream = io.BytesIO(b"synthetic-private" * 10000) error = urllib.error.HTTPError(CREDS["endpoint"], 403, "denied", {}, stream) - with patch.object(dh._opener, "open", side_effect=error), pytest.raises(dh.ResponseTooLarge) as exc: - dh.http_json("GET", CREDS["endpoint"]) + with patch.object(dh._opener, "open", side_effect=error): + status, payload = dh.http_json("GET", CREDS["endpoint"]) + # The real HTTP status survives; the oversized body is reported as such, never echoed. + assert status == 403 and payload == {"error": f"error body exceeded {dh.MAX_ERROR_BYTES} bytes"} assert stream.closed - assert "synthetic-private" not in str(exc.value) + assert "synthetic-private" not in json.dumps(payload) @pytest.mark.parametrize("entry", [None, {}, {"token": "synthetic"}, []]) -def test_explicit_missing_instance_cannot_borrow_default_credentials(entry): - with patch.object(dh, "_load_secret_map", return_value={"7": entry, "prometheus": CREDS}): +def test_explicit_missing_instance_cannot_borrow_a_sibling_default_mirror(entry): + mirror = {**CREDS, "instanceId": "3"} # the default is instance 3, not the requested 7 + with patch.object(dh, "_load_secret_map", return_value={"7": entry, "prometheus": mirror}): with pytest.raises(dh.NotConnected): dh.load_datasource("prometheus", instance_id=7) - assert dh.load_datasource("prometheus") == CREDS + assert dh.load_datasource("prometheus") == mirror + + +def test_explicit_default_instance_resolves_its_own_stamped_mirror(): + mirror = {**CREDS, "instanceId": "3"} + with patch.object(dh, "_load_secret_map", return_value={"prometheus": mirror}): + assert dh.load_datasource("prometheus", instance_id=3) == mirror + assert dh.load_datasource("prometheus", instance_id="3") == mirror + + +def test_unstamped_legacy_mirror_keeps_a_migrated_default_working_with_a_warning(capsys): + # A mirror written before the instanceId stamp (migrated default whose id key was never + # written) is the compat window; it closes on that default's next save. + with patch.object(dh, "_load_secret_map", return_value={"prometheus": CREDS}): + assert dh.load_datasource("prometheus", instance_id=7) == CREDS + assert "unstamped kind mirror" in capsys.readouterr().out @pytest.mark.parametrize("kind", ["prometheus", "mimir"]) @@ -227,3 +245,62 @@ def test_loki_malformed_siblings_preserve_valid_logs_as_unverified(): result = json.loads(module.loki_query({"query": "synthetic"})["body"]) assert result["result"] == [row] assert result["collectionStatus"] == "unknown" + + +def test_connect_headers_and_body_share_one_request_deadline(): + # Old behaviour granted the body read a fresh full `timeout` after headers arrived, so a + # slow-but-alive upstream could hold the connector for ~2x the aligned HTTP timeout and hit the + # Lambda wall. One monotonic deadline now spans open() and the body read. + clock = {"now": 1000.0} + body = iter([b"{", b"}", b""]) + + class Slow(Response): + def read1(self, n=-1): + clock["now"] += 4 # each chunk arrives 4s apart + return next(body) + + def slow_open(req, timeout=None): + clock["now"] += timeout - 1 # connect + headers consumed almost the whole budget + return Slow() + + with patch.object(dh.time, "monotonic", side_effect=lambda: clock["now"]), \ + patch.object(dh._opener, "open", side_effect=slow_open), pytest.raises(TimeoutError): + dh.http_json("GET", CREDS["endpoint"], timeout=10) + + +def test_loki_limit_saturation_is_disclosed_as_partial(): + module = importlib.import_module("loki_mcp") + hit = {"status": "success", "data": {"resultType": "streams", "result": [{"stream": {"a": "b"}, "values": [["1", "line"]]}]}} + with patch.object(module, "load_datasource", return_value=CREDS), patch.object(module, "assert_host_allowed"), \ + patch.object(module, "http_json", return_value=(200, hit)): + one = json.loads(module.loki_query_range({"query": '{a="b"}', "limit": 1})["body"]) + many = json.loads(module.loki_query_range({"query": '{a="b"}', "limit": 100})["body"]) + assert one["collectionStatus"] == "partial" and one["limitSaturated"] is True and one["truncated"] is False + assert (one["requestedLimit"], one["returnedCount"]) == (1, 1) + assert many["collectionStatus"] == "ok" and many["limitSaturated"] is False + + +def test_jaeger_limit_saturation_is_partial_but_not_loss(): + module = importlib.import_module("jaeger_mcp") + trace = {"traceID": "a" * 16, "spans": [{"startTime": 1, "duration": 1, "operationName": "op", "processID": "p1"}], + "processes": {"p1": {"serviceName": "svc"}}} + with patch.object(module, "load_datasource", return_value=CREDS), patch.object(module, "assert_host_allowed"), \ + patch.object(module, "http_json", return_value=(200, {"data": [trace]})): + one = json.loads(module.jaeger_search({"query": "service=svc&limit=1"})["body"]) + many = json.loads(module.jaeger_search({"query": "service=svc&limit=20"})["body"]) + assert one["collectionStatus"] == "partial" and one["limitSaturated"] is True and one["truncated"] is False + assert many["collectionStatus"] == "ok" and many["limitSaturated"] is False + assert (many["requestedLimit"], many["returnedCount"]) == (20, 1) + + +@pytest.mark.parametrize("kind", ["prometheus", "mimir", "loki", "tempo", "jaeger", "dynatrace", "datadog", "clickhouse"]) +def test_connection_probe_reports_latency_and_fixed_sentences(kind): + # The Test button contract is {ok, verified, latency_ms, error?}; a refused query must never echo + # upstream text, and an SSRF policy block keeps its own classification. + module = importlib.import_module(kind + "_mcp") + with patch.object(module, "load_datasource", return_value={**CREDS, "endpoint": "https://169.254.169.254"}), \ + patch.object(module, "assert_host_allowed", side_effect=dh.SsrfBlocked("blocked")), \ + patch.object(module, "http_json", side_effect=AssertionError("must not be called")): + probe = json.loads(getattr(module, kind + "_health")({})["body"]) + assert probe["ok"] is False and probe["verified"] == [] and isinstance(probe["latency_ms"], int) + assert probe["error"] == "endpoint rejected by the private-network policy" diff --git a/web/app/api/datasources/manage/route.test.ts b/web/app/api/datasources/manage/route.test.ts index 775e4765a..dd08dea44 100644 --- a/web/app/api/datasources/manage/route.test.ts +++ b/web/app/api/datasources/manage/route.test.ts @@ -72,7 +72,9 @@ describe('POST create', () => { expect(resp.status).toBe(201); expect(await resp.json()).toEqual({ id: 7 }); expect(setIntegrationCredentialById).toHaveBeenCalledWith(7, { endpoint: 'http://10.0.0.5:9090', authType: 'bearer', token: 't' }); - expect(mirrorDefaultCredential).toHaveBeenCalledWith('prometheus', { endpoint: 'http://10.0.0.5:9090', authType: 'bearer', token: 't' }); + // the default mirror is stamped with the instance id so the connector can tell this default's + // id-keyed lookup apart from a sibling instance borrowing the mirror + expect(mirrorDefaultCredential).toHaveBeenCalledWith('prometheus', { endpoint: 'http://10.0.0.5:9090', authType: 'bearer', token: 't' }, undefined, 7); }); it('fires a best-effort connect-time introspect (after the 201) and never lets it fail create', async () => { diff --git a/web/app/api/datasources/manage/route.ts b/web/app/api/datasources/manage/route.ts index 051cae4d5..8fad400fe 100644 --- a/web/app/api/datasources/manage/route.ts +++ b/web/app/api/datasources/manage/route.ts @@ -104,7 +104,7 @@ export async function POST(request: Request) { const id = await createDatasource({ name, kind, endpoint, authType: authType as 'none', settings }); await setIntegrationCredentialById(id, blob); const ds = await getDatasource(id); - if (ds?.isDefault) await mirrorDefaultCredential(kind, blob); // first of its kind → it is the default + if (ds?.isDefault) await mirrorDefaultCredential(kind, blob, undefined, id); // first of its kind → it is the default warmSchemaCache(id, kind, blob as ConnConfig); // §3.C connect-time introspect (best-effort, after response) return json({ id }, 201); } catch (e) { @@ -223,7 +223,7 @@ export async function PATCH(request: Request) { // updateDatasource (below) re-mirrors from the freshly written id blob for a default // instance; this explicit refresh keeps the mirror correct even when the row update has // nothing to change. - if (ds.isDefault) await mirrorDefaultCredential(ds.kind, blob, client); + if (ds.isDefault) await mirrorDefaultCredential(ds.kind, blob, client, id); // A database change (set OR clear) re-grounds AI query generation — mirror POST's // connect-time warm (best-effort; the 6h isSchemaStale refresh remains). if (settings !== undefined && (settings.database ?? null) !== (ds.settings?.database ?? null)) { diff --git a/web/lib/datasources.test.ts b/web/lib/datasources.test.ts index 901ec1ce8..7ac7d2b52 100644 --- a/web/lib/datasources.test.ts +++ b/web/lib/datasources.test.ts @@ -103,7 +103,7 @@ describe('updateDatasource', () => { query.mockResolvedValueOnce({ rows: [{ id: 9, name: 'n', kind: 'prometheus', endpoint: 'http://p', ds_auth_type: 'none', is_default: true, enabled: true }] }); // re-read getCredentialById.mockResolvedValueOnce({ endpoint: 'http://p', authType: 'none' }); await updateDatasource(9, { endpoint: 'http://p' }); - expect(mirrorDefaultCredential).toHaveBeenCalledWith('prometheus', { endpoint: 'http://p', authType: 'none' }, undefined); + expect(mirrorDefaultCredential).toHaveBeenCalledWith('prometheus', { endpoint: 'http://p', authType: 'none' }, undefined, expect.any(Number)); }); it('does NOT mirror when the updated row is not the default', async () => { @@ -138,7 +138,7 @@ describe('resolveConnConfig', () => { expect(getCredentialById).not.toHaveBeenCalledWith(1, 'prometheus'); // never the kind fallback }); - it('clickhouse settings ride the conn config (database + timeoutS); other kinds never set them', async () => { + it('timeoutS rides the conn config for every kind (one bound on every path); database stays ClickHouse-only', async () => { getCredentialById.mockResolvedValueOnce(null); const ch = { ...row, kind: 'clickhouse', settings: { database: 'metrics_db', timeoutS: 30 } }; expect(await resolveConnConfig(ch)).toMatchObject({ database: 'metrics_db', timeoutS: 30 }); @@ -146,7 +146,9 @@ describe('resolveConnConfig', () => { const prom = { ...row, settings: { database: 'metrics_db', timeoutS: 30 } }; // kind: prometheus const cc = await resolveConnConfig(prom); expect(cc).not.toHaveProperty('database'); - expect(cc).not.toHaveProperty('timeoutS'); + // the credential-blind agent/worker path reads timeoutS from the secret blob; the console path + // must hand the connector the same bound instead of silently falling back to its default + expect(cc).toMatchObject({ timeoutS: 30 }); }); it('a stale blob database/timeoutS never leaks through — the ROW settings are authoritative', async () => { diff --git a/web/lib/datasources.ts b/web/lib/datasources.ts index fb7d48576..c0a91cdf1 100644 --- a/web/lib/datasources.ts +++ b/web/lib/datasources.ts @@ -184,12 +184,13 @@ export async function resolveConnConfig(ds: DatasourceRow): Promise ...cred, ...(ds.endpoint ? { endpoint: ds.endpoint } : {}), ...(ds.authType ? { authType: ds.authType } : {}), - // gap L203: the ClickHouse settings ride the conn config — database (identifier-validated - // on write AND read) becomes &database=, and timeoutS becomes the connector's DEFAULT - // max_execution_time, so the Explore route, the service-graph sources, and the agent - // path all get the same bound from one mechanism. + // gap L203: settings ride the conn config so the Explore route, the service-graph sources and + // the credential-blind agent/worker path (which reads the same keys from the secret blob) all get + // ONE bound from one mechanism. database (identifier-validated on write AND read) is + // ClickHouse-only; timeoutS goes to every kind — Prometheus/Mimir/ClickHouse apply it as the + // provider execution ceiling, the other kinds as the transport deadline. ...(ds.kind === 'clickhouse' && ds.settings?.database ? { database: ds.settings.database } : {}), - ...(ds.kind === 'clickhouse' && ds.settings?.timeoutS ? { timeoutS: ds.settings.timeoutS } : {}), + ...(ds.settings?.timeoutS ? { timeoutS: ds.settings.timeoutS } : {}), } as ConnConfig; } @@ -227,7 +228,7 @@ export async function updateDatasource( const row = await getDatasource(id, q); if (row?.isDefault) { const cred = await getCredentialById(id, row.kind); - if (cred) await mirrorDefaultCredential(row.kind, cred, q === getPool() ? undefined : q); + if (cred) await mirrorDefaultCredential(row.kind, cred, q === getPool() ? undefined : q, id); } } @@ -253,7 +254,7 @@ export async function setDefaultDatasource(id: number): Promise { } // Secrets Manager write happens on its own connection/lock — after the DB commit. const cred = await getCredentialById(id, kind); - if (cred) await mirrorDefaultCredential(kind, cred); + if (cred) await mirrorDefaultCredential(kind, cred, undefined, id); } /** Delete a datasource instance. Cascade order: schema-cache rows → credential id key → @@ -286,7 +287,7 @@ export async function deleteDatasource(id: number): Promise { const newId = rows[0].id as number; await getPool().query('UPDATE integrations SET is_default = true, updated_at = NOW() WHERE id = $1', [newId]); const cred = await getCredentialById(newId, row.kind); - if (cred) await mirrorDefaultCredential(row.kind, cred); + if (cred) await mirrorDefaultCredential(row.kind, cred, undefined, newId); } else { try { await deleteCredentialKeys([row.kind]); // no instances left → clear the managed mirror diff --git a/web/lib/integration-credentials.id.test.ts b/web/lib/integration-credentials.id.test.ts index 33189a1fb..869001409 100644 --- a/web/lib/integration-credentials.id.test.ts +++ b/web/lib/integration-credentials.id.test.ts @@ -94,6 +94,13 @@ describe('mirrorDefaultCredential', () => { expect(w['3']).toEqual({ endpoint: 'http://id3' }); expect(w['loki']).toEqual({ endpoint: 'http://loki', authType: 'bearer', token: 't' }); }); + + it('stamps the mirror with the default instance id when given one', async () => { + smSend.mockImplementation((cmd: any) => (cmd.kind === 'get' ? getReturn({}) : {})); + await mirrorDefaultCredential('loki', { endpoint: 'http://loki', authType: 'none' }, undefined, 3); + expect(writtenMap()['loki']).toEqual({ endpoint: 'http://loki', authType: 'none', instanceId: '3' }); + await expect(mirrorDefaultCredential('loki', { endpoint: 'http://loki' }, undefined, 0)).rejects.toThrow(/invalid integration id/); + }); }); describe('getConfiguredIds', () => { diff --git a/web/lib/integration-credentials.ts b/web/lib/integration-credentials.ts index 37e96f209..20c630b66 100644 --- a/web/lib/integration-credentials.ts +++ b/web/lib/integration-credentials.ts @@ -167,15 +167,19 @@ export async function setIntegrationCredentialById( }, q); } -/** Write the managed default-mirror under the plain `kind` key (agent gateway no-inline path). */ +/** Write the managed default-mirror under the plain `kind` key (agent gateway no-inline path). + * The mirror is stamped with the default instance's id (`instanceId`) so the connector's + * id-keyed lookup can accept the mirror for THAT instance only, never for a sibling. */ export async function mirrorDefaultCredential( kind: string, secretObj: Record, q?: { query: (text: string, values?: unknown[]) => Promise }, + instanceId?: number, ): Promise { assertKnownSlug(kind); + if (instanceId !== undefined) assertPositiveId(instanceId); await mutateCredentialMap((map) => { - map[kind] = secretObj; + map[kind] = instanceId === undefined ? secretObj : { ...secretObj, instanceId: String(instanceId) }; }, q); } From 06759fdd097aaae8456877eeef974fc371726f9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=98=A4=EC=A4=80=EC=84=9D=28Junseok=20Oh=29DevOps?= <52226147+Atom-oh@users.noreply.github.com> Date: Sun, 18 Oct 2026 14:28:10 +0000 Subject: [PATCH 03/27] test(web): expect the stamped instance id on default re-mirrors Co-Authored-By: Claude Fable 5.1 --- web/lib/datasources.default.test.ts | 2 +- web/lib/datasources.delete.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/web/lib/datasources.default.test.ts b/web/lib/datasources.default.test.ts index f1ee74a75..548022428 100644 --- a/web/lib/datasources.default.test.ts +++ b/web/lib/datasources.default.test.ts @@ -40,7 +40,7 @@ describe('setDefaultDatasource', () => { expect(stmts.some((s) => /is_default\s*=\s*true/i.test(s) && /id\s*=\s*\$1/.test(s))).toBe(true); expect(stmts.some((s) => /COMMIT/i.test(s))).toBe(true); expect(clientRelease).toHaveBeenCalled(); - expect(mirrorDefaultCredential).toHaveBeenCalledWith('prometheus', { endpoint: 'http://p', authType: 'none' }); + expect(mirrorDefaultCredential).toHaveBeenCalledWith('prometheus', { endpoint: 'http://p', authType: 'none' }, undefined, 12); }); it('rolls back and throws when the datasource does not exist', async () => { diff --git a/web/lib/datasources.delete.test.ts b/web/lib/datasources.delete.test.ts index f59994300..5e123e58d 100644 --- a/web/lib/datasources.delete.test.ts +++ b/web/lib/datasources.delete.test.ts @@ -58,7 +58,7 @@ describe('deleteDatasource', () => { .mockResolvedValueOnce({ rows: [] }); // UPDATE set new default getCredentialById.mockResolvedValueOnce({ endpoint: 'http://p8', authType: 'none' }); await deleteDatasource(5); - expect(mirrorDefaultCredential).toHaveBeenCalledWith('prometheus', { endpoint: 'http://p8', authType: 'none' }); + expect(mirrorDefaultCredential).toHaveBeenCalledWith('prometheus', { endpoint: 'http://p8', authType: 'none' }, undefined, expect.any(Number)); }); it('clears the kind mirror when no instances of the kind remain', async () => { From 55b8c7d0aac4b0b5d5eb3a39d6fa716a547749c5 Mon Sep 17 00:00:00 2001 From: atomoh Date: Sun, 18 Oct 2026 14:44:33 +0000 Subject: [PATCH 04/27] fix: bind datasource credentials to their registered identity --- agent/lambda/datasource_http.py | 13 +++--- agent/lambda/test_external_connections.py | 15 +++++-- .../datasource-credential-isolation.md | 44 +++++++++++++++++++ web/app/api/datasources/manage/route.test.ts | 2 + web/app/api/datasources/manage/route.ts | 3 +- web/lib/datasources.default.test.ts | 12 +++++ web/lib/datasources.delete.test.ts | 11 +++++ web/lib/datasources.ts | 2 + web/lib/integration-credentials.id.test.ts | 32 +++++++++++++- web/lib/integration-credentials.ts | 19 ++++++-- 10 files changed, 134 insertions(+), 19 deletions(-) create mode 100644 docs/runbooks/datasource-credential-isolation.md diff --git a/agent/lambda/datasource_http.py b/agent/lambda/datasource_http.py index dd0684b90..4d37e1da2 100644 --- a/agent/lambda/datasource_http.py +++ b/agent/lambda/datasource_http.py @@ -293,10 +293,10 @@ def load_datasource(slug, instance_id=None): where the caller sends ONLY an instance_id and the connector reads the secret server-side; 3. the `map[slug]` kind-mirror: when no explicit instance was requested, or when the requested id IS the default instance the mirror was written for (`instanceId` stamp). - An explicit id never inherits a mirror stamped for a different instance. A mirror without a - stamp predates the stamp (a migrated default whose id key was never written); it is accepted - for an explicit id with a warning so that default keeps working until its next save writes the - id key and the stamp. + An explicit id never inherits an unstamped mirror or one stamped for another instance. + Legacy defaults must be re-saved through the verified-current-default administrator path + before this credential-blind producer rolls out; that path writes the id key and stamp. + Calls without an explicit id retain the legacy kind-default lookup. `instance_id` is a pure argument (no module-global), so warm-container reuse cannot bleed one invocation's instance into the next.""" if _REQUEST_CONN is not None: @@ -308,10 +308,7 @@ def load_datasource(slug, instance_id=None): mirror = m.get(slug) stamp = mirror.get("instanceId") if isinstance(mirror, dict) else None if isinstance(mirror, dict) and mirror.get("endpoint") and ( - stamp is None or str(stamp) == str(instance_id)): - if stamp is None: - print(f"[datasource_http] WARNING: {slug} instance {instance_id} resolved via an " - "unstamped kind mirror; re-save the default instance to write its id key") + stamp is not None and str(stamp) == str(instance_id)): creds = mirror else: raise NotConnected("requested datasource instance is not configured") diff --git a/agent/lambda/test_external_connections.py b/agent/lambda/test_external_connections.py index 026e9397c..e4928f37f 100644 --- a/agent/lambda/test_external_connections.py +++ b/agent/lambda/test_external_connections.py @@ -67,12 +67,19 @@ def test_explicit_default_instance_resolves_its_own_stamped_mirror(): assert dh.load_datasource("prometheus", instance_id="3") == mirror -def test_unstamped_legacy_mirror_keeps_a_migrated_default_working_with_a_warning(capsys): - # A mirror written before the instanceId stamp (migrated default whose id key was never - # written) is the compat window; it closes on that default's next save. +def test_unstamped_legacy_mirror_cannot_authorize_an_explicit_instance(): with patch.object(dh, "_load_secret_map", return_value={"prometheus": CREDS}): + with pytest.raises(dh.NotConnected, match="requested datasource instance"): + dh.load_datasource("prometheus", instance_id=7) + assert dh.load_datasource("prometheus") == CREDS + + +def test_backfilled_legacy_default_remains_readable_without_lending_to_siblings(): + mirror = {**CREDS, "instanceId": "7"} + with patch.object(dh, "_load_secret_map", return_value={"7": CREDS, "prometheus": mirror}): assert dh.load_datasource("prometheus", instance_id=7) == CREDS - assert "unstamped kind mirror" in capsys.readouterr().out + with pytest.raises(dh.NotConnected): + dh.load_datasource("prometheus", instance_id=8) @pytest.mark.parametrize("kind", ["prometheus", "mimir"]) diff --git a/docs/runbooks/datasource-credential-isolation.md b/docs/runbooks/datasource-credential-isolation.md new file mode 100644 index 000000000..f1548d6af --- /dev/null +++ b/docs/runbooks/datasource-credential-isolation.md @@ -0,0 +1,44 @@ +# Datasource credential identity and rollout + +Native datasource connectors store one credential blob per integration ID. +Their plain kind key is a managed mirror for the current default. The mirror +includes `instanceId`; an explicit-ID producer read accepts only its own ID key +or a mirror bearing that exact ID. Missing credentials fail closed. Calls without +an explicit ID retain the existing kind-default lookup. + +An old unstamped mirror cannot prove which instance owns its credentials. +Before deploying the stricter connector producer, operators must backfill every +migrated default through the existing authenticated administrator save flow: + +1. Identify the current default datasource in the Integrations interface. Keep + its endpoint and authentication type unchanged. +2. Save that datasource's connection settings. The server re-reads the row under + its manage lock; only a current default may recover an unstamped kind mirror, + and its stored endpoint must exactly match that row. Do not print credentials. +3. Confirm the save succeeded. It writes the instance ID credential and stamps + the kind mirror. A settings-only save preserves the matched authentication + material. A mismatched stamp or endpoint is rejected as a credential source; + resupply the intended instance's credentials through the same write-only form. +4. Verify a credential-blind read for that exact instance ID before the connector + rollout, and verify a same-kind instance without its own credentials fails + closed. Stop deployment until all migrated defaults pass. + +Changing or deleting a default does not establish ownership of its old mirror. +Only an ID-bound credential may be mirrored to the replacement; absent credentials +clear the kind mirror instead of lending the previous default's authentication. +Resave migrated defaults before switching them. Secrets Manager and row writes are +not one atomic cross-service transaction; a failed save requires inspection and +retry through the administrator flow before rollout. + +The implementation is `web/lib/integration-credentials.ts`, +`web/app/api/datasources/manage/route.ts`, `web/lib/datasources.ts` and +`agent/lambda/datasource_http.py`. Focused verification: + +```bash +cd web && npx vitest run lib/integration-credentials.id.test.ts lib/datasources.test.ts lib/datasources.default.test.ts lib/datasources.delete.test.ts app/api/datasources/manage/route.test.ts +cd agent/lambda && python3 -m pytest test_external_connections.py -q +``` + +Run the two commands from the repository root independently. These local tests +mock Secrets Manager/provider reads and do not establish successful production +backfill, producer deployment, IAM readiness or live provider access. diff --git a/web/app/api/datasources/manage/route.test.ts b/web/app/api/datasources/manage/route.test.ts index dd08dea44..f69dddac8 100644 --- a/web/app/api/datasources/manage/route.test.ts +++ b/web/app/api/datasources/manage/route.test.ts @@ -236,6 +236,7 @@ describe('PATCH update', () => { getDatasource.mockResolvedValue({ id: 7, kind: 'prometheus', endpoint: 'http://p:9090', authType: 'bearer', isDefault: true, settings: {} }); const { PATCH } = await import('./route'); await PATCH(req({ id: 7, settings: { timeoutS: 15 } }, 'PATCH')); + expect(getCredentialById).toHaveBeenCalledWith(7, 'prometheus', 'http://p:9090'); const blob = setIntegrationCredentialById.mock.calls.at(-1)![1]; expect(blob.token).toBe('tok'); // NOT de-authenticated expect(blob.timeoutS).toBe(15); @@ -244,6 +245,7 @@ describe('PATCH update', () => { expect(mirrored[0]).toBe('prometheus'); expect(mirrored[1].token).toBe('tok'); expect(mirrored[1].timeoutS).toBe(15); + expect(mirrored[3]).toBe(7); }); it('a backslash endpoint is rejected on PATCH (URL-parser differential, round-6)', async () => { diff --git a/web/app/api/datasources/manage/route.ts b/web/app/api/datasources/manage/route.ts index 8fad400fe..dadcec332 100644 --- a/web/app/api/datasources/manage/route.ts +++ b/web/app/api/datasources/manage/route.ts @@ -178,7 +178,8 @@ export async function PATCH(request: Request) { // Merge base: for a migrated DEFAULT instance the credential can live only under the // kind mirror (round-4 gate) — an id-only read would come back empty and a settings-only // PATCH would de-authenticate the instance AND clobber the mirror the agent path reads. - const existing: Record = { ...((await getCredentialById(id, ds.isDefault ? ds.kind : undefined)) ?? {}) }; + const existing: Record = { ...((await getCredentialById(id, + ds.isDefault ? ds.kind : undefined, ds.isDefault ? ds.endpoint ?? undefined : undefined)) ?? {}) }; // Settings keys are stripped UNCONDITIONALLY (the row is authoritative; an endpoint-only // PATCH must not carry a historical stale timeoutS/database forward either). delete existing.timeoutS; diff --git a/web/lib/datasources.default.test.ts b/web/lib/datasources.default.test.ts index 548022428..27a9946a0 100644 --- a/web/lib/datasources.default.test.ts +++ b/web/lib/datasources.default.test.ts @@ -6,9 +6,11 @@ const poolConnect = vi.fn(); vi.mock('@/lib/db', () => ({ getPool: () => ({ connect: poolConnect, query: clientQuery }) })); const getCredentialById = vi.fn(); const mirrorDefaultCredential = vi.fn(); +const deleteCredentialKeys = vi.fn(); vi.mock('@/lib/integration-credentials', () => ({ getCredentialById: (...a: unknown[]) => getCredentialById(...a), mirrorDefaultCredential: (...a: unknown[]) => mirrorDefaultCredential(...a), + deleteCredentialKeys: (...a: unknown[]) => deleteCredentialKeys(...a), })); import { setDefaultDatasource } from './datasources'; @@ -19,9 +21,19 @@ beforeEach(() => { poolConnect.mockReset().mockResolvedValue({ query: clientQuery, release: clientRelease }); getCredentialById.mockReset(); mirrorDefaultCredential.mockReset(); + deleteCredentialKeys.mockReset().mockResolvedValue(undefined); }); describe('setDefaultDatasource', () => { + it('clears the former mirror when the chosen default has no bound credential', async () => { + clientQuery.mockResolvedValueOnce({ rows: [] }) + .mockResolvedValueOnce({ rows: [{ kind: 'prometheus' }] }); + getCredentialById.mockResolvedValueOnce(null); + await setDefaultDatasource(12); + expect(getCredentialById).toHaveBeenCalledWith(12, 'prometheus'); + expect(mirrorDefaultCredential).not.toHaveBeenCalled(); + expect(deleteCredentialKeys).toHaveBeenCalledWith(['prometheus']); + }); it('unsets other defaults of the kind then sets this one, in a transaction, and re-mirrors', async () => { // BEGIN, SELECT kind, UPDATE others false, UPDATE this true, COMMIT clientQuery diff --git a/web/lib/datasources.delete.test.ts b/web/lib/datasources.delete.test.ts index 5e123e58d..d3f2319a8 100644 --- a/web/lib/datasources.delete.test.ts +++ b/web/lib/datasources.delete.test.ts @@ -25,6 +25,17 @@ beforeEach(() => { }); describe('deleteDatasource', () => { + it('clears the deleted default mirror when its successor has no bound credential', async () => { + query.mockImplementation(async (sql: string) => ({ + rows: sql.includes('SELECT id, name') ? [row({ id: 5, is_default: true })] : + sql.includes('SELECT id FROM') ? [{ id: 8 }] : [], + })); + getCredentialById.mockResolvedValueOnce(null); + await deleteDatasource(5); + expect(getCredentialById).toHaveBeenCalledWith(8, 'prometheus'); + expect(mirrorDefaultCredential).not.toHaveBeenCalled(); + expect(deleteCredentialKeys).toHaveBeenCalledWith(['prometheus']); + }); it('is a no-op when the id does not exist', async () => { query.mockResolvedValueOnce({ rows: [] }); // getDatasource → none await deleteDatasource(404); diff --git a/web/lib/datasources.ts b/web/lib/datasources.ts index c0a91cdf1..24eb318e3 100644 --- a/web/lib/datasources.ts +++ b/web/lib/datasources.ts @@ -255,6 +255,7 @@ export async function setDefaultDatasource(id: number): Promise { // Secrets Manager write happens on its own connection/lock — after the DB commit. const cred = await getCredentialById(id, kind); if (cred) await mirrorDefaultCredential(kind, cred, undefined, id); + else await deleteCredentialKeys([kind]); // never retain the previous default's credentials } /** Delete a datasource instance. Cascade order: schema-cache rows → credential id key → @@ -288,6 +289,7 @@ export async function deleteDatasource(id: number): Promise { await getPool().query('UPDATE integrations SET is_default = true, updated_at = NOW() WHERE id = $1', [newId]); const cred = await getCredentialById(newId, row.kind); if (cred) await mirrorDefaultCredential(row.kind, cred, undefined, newId); + else await deleteCredentialKeys([row.kind]); // the removed default cannot authenticate its successor } else { try { await deleteCredentialKeys([row.kind]); // no instances left → clear the managed mirror diff --git a/web/lib/integration-credentials.id.test.ts b/web/lib/integration-credentials.id.test.ts index 869001409..d949caaee 100644 --- a/web/lib/integration-credentials.id.test.ts +++ b/web/lib/integration-credentials.id.test.ts @@ -74,9 +74,37 @@ describe('getCredentialById', () => { expect(await getCredentialById(5, 'prometheus')).toEqual({ endpoint: 'http://id5' }); }); - it('falls back to the kind mirror when the id entry is absent', async () => { + it('does not lend an unstamped kind mirror to an explicit instance', async () => { smSend.mockImplementation(() => getReturn({ prometheus: { endpoint: 'http://def' } })); - expect(await getCredentialById(99, 'prometheus')).toEqual({ endpoint: 'http://def' }); + expect(await getCredentialById(99, 'prometheus')).toBeNull(); + }); + + it('uses only a mirror stamped for the requested id', async () => { + const mirror = { endpoint: 'http://def', instanceId: '5' }; + smSend.mockImplementation(() => getReturn({ prometheus: mirror })); + expect(await getCredentialById(5, 'prometheus')).toEqual(mirror); + expect(await getCredentialById(99, 'prometheus', 'http://def')).toBeNull(); + }); + + it('recovers a legacy current default only at its verified exact endpoint', async () => { + smSend.mockImplementation(() => getReturn({ prometheus: { endpoint: 'http://def', token: 'fixture' } })); + expect(await getCredentialById(5, 'prometheus', 'http://def')).toEqual({ endpoint: 'http://def', token: 'fixture' }); + expect(await getCredentialById(5, 'prometheus', 'http://def/another-tenant')).toBeNull(); + }); + + it('backfills a verified legacy default with no sibling credential inheritance', async () => { + let map: Record = { prometheus: { endpoint: 'http://def', token: 'fixture' } }; + smSend.mockImplementation((cmd: any) => { + if (cmd.kind === 'get') return getReturn(map); + map = JSON.parse(cmd.input.SecretString); + return {}; + }); + const old = await getCredentialById(5, 'prometheus', 'http://def'); + await setIntegrationCredentialById(5, old!); + await mirrorDefaultCredential('prometheus', old!, undefined, 5); + expect(await getCredentialById(5)).toEqual(old); + expect(await getCredentialById(99, 'prometheus')).toBeNull(); + expect(map.prometheus).toMatchObject({ token: 'fixture', instanceId: '5' }); }); it('returns null when neither id nor kind mirror exists', async () => { diff --git a/web/lib/integration-credentials.ts b/web/lib/integration-credentials.ts index 20c630b66..b0b7778ff 100644 --- a/web/lib/integration-credentials.ts +++ b/web/lib/integration-credentials.ts @@ -183,18 +183,29 @@ export async function mirrorDefaultCredential( }, q); } -/** Resolve an instance's credential: the id entry, else the kind mirror (fallback), else null. - * Throws on a Secrets Manager read failure (the caller — query/test — surfaces it). */ +/** Resolve the id entry or a mirror stamped for this exact id. An unstamped legacy mirror + * is recoverable only during an administrator save of the verified CURRENT default row, + * with that row's exact endpoint passed by server code. Default switching/deletion cannot + * establish this historical identity. Throws on Secrets Manager read failure. */ export async function getCredentialById( id: number, fallbackKind?: string, + legacyDefaultEndpoint?: string, ): Promise | null> { + assertPositiveId(id); const map = await readMap(); const byId = map[String(id)]; - if (byId && typeof byId === 'object') return byId as Record; + if (byId && typeof byId === 'object' && !Array.isArray(byId)) return byId as Record; if (fallbackKind) { const byKind = map[fallbackKind]; - if (byKind && typeof byKind === 'object') return byKind as Record; + if (byKind && typeof byKind === 'object' && !Array.isArray(byKind)) { + const mirror = byKind as Record; + if (mirror.instanceId !== undefined && mirror.instanceId !== null) { + if (String(mirror.instanceId) === String(id)) return mirror; + } else if (legacyDefaultEndpoint && mirror.endpoint === legacyDefaultEndpoint) { + return mirror; + } + } } return null; } From 4f061be166d96eead6803e65d123b22f680b9cd9 Mon Sep 17 00:00:00 2001 From: atomoh Date: Sun, 18 Oct 2026 23:00:08 +0000 Subject: [PATCH 05/27] fix: address review feedback (iteration 2/5) --- CHANGELOG.md | 2 + agent/lambda/datasource_http.py | 34 +++++++- agent/lambda/jaeger_mcp.py | 11 +-- agent/lambda/loki_mcp.py | 6 +- agent/lambda/test_external_connections.py | 80 +++++++++++++++++++ docs/runbooks/AGENTS.md | 4 +- docs/runbooks/CLAUDE.md | 1 + .../datasource-credential-isolation.md | 12 +++ web/app/api/datasources/manage/route.test.ts | 10 +++ web/app/api/datasources/manage/route.ts | 1 + web/lib/datasources.ts | 2 +- 11 files changed, 152 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 000e20d3c..e887333a0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- Native observability connectors: connection tests verify bounded query access instead of public health endpoints. Explicit instance reads require ID-bound credentials; migrated defaults must be backfilled through the administrator save flow before rollout. Prometheus and Mimir default to a 10-second execution ceiling, configurable per datasource and tightened by Explore; provider response bytes and body reads are bounded. Valid short Jaeger trace IDs are retained, and local loss, search saturation and unknown results carry distinct completeness evidence. - Security scanning hardening: deployment workflows select only explicit preview secrets; chat history deletion hides internal errors and session IDs always use cryptographic randomness. The configurator escapes literal HCL strings, screenshot login requires exact configured origins and valid TLS, presenter titles retain text semantics, datasource hostname classification checks the AWS DNS suffix including root-dot and China-partition forms, and rotation diagnostics omit unrelated secret identifiers. - AgentCore Gateway inbound authentication: the provisioner creates every section gateway with IAM (SigV4) inbound authentication instead of none. Because a gateway's authorizer type cannot be changed after creation, `make agentcore` fails on any existing gateway that still accepts unauthenticated calls, and the AgentCore reference documents the operator replacement procedure. - AI chat grounding: the chat agent is told the current time, the account and region its tools read, and how to treat capped, unfinished or failed results. Capped AWS list results report how many items were returned and found and whether more exist, a list is called complete only when its tool says so, and a successful empty result is reported as empty. Unfinished or failed Logs Insights queries, AWS-capped route searches, ECS lists with failed descriptions, unreadable serverless-cache listings and Cost Explorer comparisons that stopped early are reported as incomplete or unknown rather than complete or empty. Synced-inventory answers state that they cover the host account and warn when another account was asked for; a requested account that is unregistered, disabled or unverifiable is named instead of silently falling back to the host account; an answer produced without live tools starts with a notice that it uses no real data; and long conversations send only the most recent turns that fit the request size. A follow-up question without a domain keyword ("why is that?", "more detail") stays with the section that answered the previous turn instead of falling back to the general Ops agent, while an explicit keyword, a pin or (with hybrid routing) a classifier choice still re-routes. On AgentCore agent answers, the agent is told which regions AWSops is configured to inventory for the target account (when that scope can be read; it is an AWSops setting, not which regions AWS has enabled), so it queries them and names the regions it checked rather than answering account-wide questions from one region. When a thread switches accounts, earlier turns are labelled with the account they were about (where that was recorded), so answers about one account are not presented as facts about another. Long answers get an explicit output budget instead of the model default, so root-cause answers are no longer cut mid-sentence, and an answer that still stops early says whether it hit the length limit or lost the connection. Synced-inventory rows returned by the inventory query tool carry their region and, for EC2, instance id, type, power state and last state change (the reader view allowlists those payload keys; the last-change time is empty for running instances), so region-scoped and running/stopped questions use recorded values instead of resource-name prefixes or an "unknown" answer. @@ -691,6 +692,7 @@ First release of the **v2 line** (versioned independently from the v1 1.x line, ### 수정 +- 외부 관측 데이터소스: 연결 테스트가 공개 상태 엔드포인트 대신 제한된 실제 조회 권한을 검증합니다. 인스턴스 ID 조회는 해당 ID의 자격 증명만 허용하며, 이전 기본 연결은 배포 전에 관리자 저장 흐름으로 자격 증명을 보완해야 합니다. Prometheus·Mimir의 기본 실행 제한은 10초이며 데이터소스별 설정과 Explore의 더 짧은 제한을 적용합니다. 응답 크기와 본문 읽기 시간을 제한하고, 짧은 Jaeger trace ID를 보존하며 실제 누락·조회 한도 도달·판별 불가 결과를 구분합니다. - 보안 검사 개선: 배포 워크플로가 명시된 미리보기 시크릿만 선택하며, 채팅 기록 삭제 오류는 내부 정보를 숨기고 세션 ID는 암호학적 난수만 사용합니다. 설정 도구는 HCL 문자열을 안전하게 인코딩하고, 스크린샷 로그인은 지정한 정확한 출처와 유효한 TLS를 요구합니다. 발표자 제목은 텍스트로 보존하며, 데이터소스 호스트 분류는 DNS 루트 점과 중국 파티션을 포함한 AWS 도메인 접미사를 확인하고, 회전 진단에서는 무관한 시크릿 식별자를 생략합니다. - AgentCore Gateway 유입 인증: 프로비저너가 모든 섹션 Gateway를 인증 없음 대신 IAM(SigV4) 유입 인증으로 생성합니다. Gateway의 인증 방식은 생성 후 바꿀 수 없으므로, 아직 인증 없는 호출을 받는 기존 Gateway가 있으면 `make agentcore`가 실패하며, 운영자 교체 절차는 AgentCore 참조 문서에 안내합니다. - AI 채팅 근거 강화: 채팅 에이전트가 현재 시각, 도구가 조회하는 계정·리전, 잘리거나 끝나지 않았거나 실패한 결과를 다루는 방법을 전달받습니다. 상한이 걸린 AWS 목록 결과는 반환·조회 개수와 추가 항목 여부를 알려 주고, 도구가 완전하다고 표시한 목록만 완전하다고 말하며, 성공했지만 비어 있는 결과는 비어 있다고 답합니다. 끝나지 않았거나 실패한 Logs Insights 쿼리, AWS가 상한을 둔 경로 검색, 상세 조회가 실패한 ECS 목록, 읽을 수 없는 서버리스 캐시 목록, 중간에 멈춘 Cost Explorer 비교는 완전하거나 비어 있다고 하지 않고 불완전하거나 알 수 없다고 보고합니다. 동기화 인벤토리 답변은 호스트 계정 기준임을 밝히며 다른 계정을 물었을 때 경고하고, 요청한 계정이 등록되지 않았거나 비활성화되었거나 확인할 수 없으면 조용히 호스트 계정으로 바꾸지 않고 그 사실을 밝히며, 실시간 도구 없이 만든 답변은 실제 데이터가 아니라는 안내로 시작하고, 긴 대화는 요청 크기에 맞는 최근 대화만 보냅니다. 도메인 키워드가 없는 후속 질문("그건 왜 그래?", "더 자세히")은 일반 Ops 에이전트로 넘어가지 않고 직전 답변을 한 섹션이 이어서 답합니다. 명시적 키워드, 섹션 고정, (하이브리드 라우팅에서는) 분류기의 선택은 이전처럼 다시 라우팅합니다. AgentCore 에이전트 답변에서는 에이전트가 AWSops가 대상 계정에 대해 수집하도록 설정된 리전 목록을(조회할 수 있을 때. AWS에서 활성화된 리전이 아니라 AWSops 설정입니다) 받아, 계정 전체 질문을 한 리전만 보고 답하지 않고 해당 리전들을 조회한 뒤 확인한 리전을 밝힙니다. 한 스레드에서 계정을 바꾸면 이전 턴에 (기록된 경우) 해당 계정이 표시되어, 다른 계정에 대한 답변을 현재 계정의 사실로 제시하지 않습니다. 긴 답변에는 모델 기본값 대신 명시적인 출력 한도를 적용해 원인 분석 답변이 문장 중간에 끊기지 않으며, 그래도 일찍 끝난 답변은 출력 길이 한도에 도달했는지 연결이 끊겼는지를 알려 줍니다. 인벤토리 조회 도구가 반환하는 동기화 행에 리전이 포함되고 EC2는 인스턴스 ID·유형·전원 상태·마지막 상태 변경 시각도 포함되어(리더 뷰가 이 페이로드 키를 허용하며 실행 중 인스턴스의 상태 변경 시각은 비어 있음), 리전 한정 질문과 실행/중지 질문이 이름 접두사 추정이나 "알 수 없음" 대신 기록된 값으로 답변됩니다. diff --git a/agent/lambda/datasource_http.py b/agent/lambda/datasource_http.py index 4d37e1da2..c4522db85 100644 --- a/agent/lambda/datasource_http.py +++ b/agent/lambda/datasource_http.py @@ -4,7 +4,10 @@ endpoint + credential queried in a query language. This module centralizes: reading the per-slug credential from the single integrations secret, SSRF host guarding (always-block metadata/loopback/...; private allowed — in-cluster datasources are the target), -auth headers (Basic/Bearer), and a no-redirect HTTP fetch. Stdlib + boto3 only. +auth headers (Basic/Bearer), and a no-redirect HTTP fetch. Explicit instance IDs require an +ID-bound credential; kind-only reads retain the default mirror. Responses are byte-bounded before +parsing and body reads share the request's monotonic deadline. Connection tests execute bounded +queries instead of trusting public health endpoints. Stdlib + boto3 only. SECURITY: never log credential material. The endpoint is user-supplied → assert_host_allowed before every request; redirects are NOT auto-followed (a malicious endpoint can't 30x to metadata/internal). @@ -231,6 +234,8 @@ def query_probe(run, error, accepted=("ok", "empty", "partial")): state = run() except SsrfBlocked: state, error = None, "endpoint rejected by the private-network policy" + except NotConnected: + state, error = None, "datasource not connected" except Exception: state = None latency = int((time.monotonic() - t0) * 1000) @@ -416,6 +421,20 @@ def _parse(raw): return {"raw": raw.decode("utf-8", "replace")[:4000]} +def _response_socket(response): + """Find urllib's HTTPResponse socket, including HTTPError's extra fp wrapper.""" + stream = response + for _ in range(4): + raw = getattr(stream, "raw", None) + sock = getattr(raw, "_sock", None) + if sock is not None: + return sock + stream = getattr(stream, "fp", None) + if stream is None: + break + return None + + def _read_bounded(response, max_bytes, deadline): """Read at most budget+1 bytes before the absolute monotonic `deadline`; never trust Content-Length or parse before the bound. The deadline is the request's — connect, headers @@ -425,9 +444,18 @@ def _read_bounded(response, max_bytes, deadline): read = getattr(response, "read1", response.read) try: while True: + remaining = deadline - time.monotonic() + if remaining <= 0: + raise TimeoutError("datasource response read deadline exceeded") + sock = _response_socket(response) + if sock is not None: + sock.settimeout(remaining) + try: + chunk = read(min(65536, max_bytes - total + 1)) + except socket.timeout as exc: + raise TimeoutError("datasource response read deadline exceeded") from exc if time.monotonic() >= deadline: raise TimeoutError("datasource response read deadline exceeded") - chunk = read(min(65536, max_bytes - total + 1)) if not chunk: return b"".join(chunks) total += len(chunk) @@ -461,4 +489,6 @@ def http_json(method, url, headers=None, body=None, timeout=HTTP_TIMEOUT, max_by return e.code, {"error": f"error body exceeded {MAX_ERROR_BYTES} bytes"} except TimeoutError: return e.code, {"error": "error body read deadline exceeded"} + except (OSError, http.client.HTTPException): + return e.code, {"error": "error body could not be read"} return e.code, _parse(raw) diff --git a/agent/lambda/jaeger_mcp.py b/agent/lambda/jaeger_mcp.py index 92eb38fac..868b059bb 100644 --- a/agent/lambda/jaeger_mcp.py +++ b/agent/lambda/jaeger_mcp.py @@ -105,7 +105,7 @@ def jaeger_search(args): invalid = False for t in traces[:MAX_TRACES]: if (not isinstance(t, dict) or not isinstance(t.get("traceID"), str) - or len(t["traceID"]) not in (16, 32) or not _HEX.fullmatch(t["traceID"]) + or not 1 <= len(t["traceID"]) <= 32 or not _HEX.fullmatch(t["traceID"]) or not isinstance(t.get("spans"), list) or not t["spans"] or not isinstance(t.get("processes"), dict)): invalid = True @@ -131,16 +131,17 @@ def jaeger_search(args): "startTime": root.get("startTime", 0), }) # Reaching the requested limit is saturation, not loss: Jaeger returns no marker, so more traces - # may exist and the result is `partial`, but only a real drop (byte bound) sets `truncated`. + # may exist and the result is `partial`; a local row/byte drop sets `truncated`. saturated = len(traces) >= requested http_status = getattr(data, "http_status", 200) - state = "unknown" if invalid or http_status not in (200, 206) else "partial" if saturated or http_status == 206 else "ok" if rows else "empty" + dropped = len(traces) > MAX_TRACES + state = "unknown" if invalid or http_status not in (200, 206) else "partial" if saturated or dropped or http_status == 206 else "ok" if rows else "empty" payload, btr = _byte_bound({"traces": rows}) if btr: payload.pop("preview", None) # never echo provider content, even as a bounded preview - return ok({**payload, "truncated": btr, "limitSaturated": saturated, + return ok({**payload, "truncated": btr or dropped, "limitSaturated": saturated, "collectionStatus": "unknown" if invalid else "partial" if btr else state, - "requestedLimit": requested, "returnedCount": len(rows)}) + "requestedLimit": requested, "returnedCount": len(payload.get("traces", []))}) def jaeger_get_trace(args): diff --git a/agent/lambda/loki_mcp.py b/agent/lambda/loki_mcp.py index 18ffa855a..99ecc8b62 100644 --- a/agent/lambda/loki_mcp.py +++ b/agent/lambda/loki_mcp.py @@ -123,8 +123,10 @@ def _query_response(data, requested=None): valid = valid and len(rows) == len(retained) bounded, truncated = _bound({"resultType": kind, "result": rows}) truncated = truncated or len(source_rows) > MAX_STREAMS - returned = sum(len(row.get("values") or []) for row in rows if isinstance(row, dict)) if kind == "streams" else len(rows) - saturated = bool(requested) and returned >= requested + upstream_lines = sum(len(row.get("values") or []) for row in rows) if kind == "streams" else 0 + emitted = bounded.get("result", []) if isinstance(bounded, dict) else [] + returned = sum(len(row.get("values") or []) for row in emitted) if kind == "streams" else len(emitted) + saturated = kind == "streams" and bool(requested) and upstream_lines >= requested status = getattr(data, "http_status", 200) state = ("unknown" if not valid or status not in (200, 206) else "partial" if truncated or saturated or status == 206 or getattr(data, "source_partial", False) else diff --git a/agent/lambda/test_external_connections.py b/agent/lambda/test_external_connections.py index e4928f37f..660f23cca 100644 --- a/agent/lambda/test_external_connections.py +++ b/agent/lambda/test_external_connections.py @@ -1,8 +1,11 @@ """Connection identity, bounded I/O, query proof and result-completion regressions.""" import importlib +import http.client import io import json +import socket import sys +import time import urllib.error from pathlib import Path from unittest.mock import patch @@ -275,6 +278,83 @@ def slow_open(req, timeout=None): dh.http_json("GET", CREDS["endpoint"], timeout=10) +@pytest.mark.parametrize("status", [200, 403]) +def test_blocking_socket_read_uses_only_budget_remaining_after_headers(status): + client, peer = socket.socketpair() + client.settimeout(1.5) + peer.sendall(f"HTTP/1.1 {status} Synthetic\r\nContent-Length: 2\r\n\r\n".encode()) + response = http.client.HTTPResponse(client) + response.begin() + + def delayed_open(req, timeout=None): + time.sleep(1.0) # Header/connect phase has already spent most of the total budget. + if status == 403: + raise urllib.error.HTTPError(CREDS["endpoint"], status, "denied", {}, response) + return response + + start = time.monotonic() + try: + with patch.object(dh._opener, "open", side_effect=delayed_open): + if status == 200: + with pytest.raises(TimeoutError): + dh.http_json("GET", CREDS["endpoint"], timeout=1.5) + else: + assert dh.http_json("GET", CREDS["endpoint"], timeout=1.5) == ( + 403, {"error": "error body read deadline exceeded"}) + # The original 1.5s socket timeout would take 2.5s including delayed headers. + assert time.monotonic() - start < 2.0 + assert response.isclosed() + finally: + response.close() + client.close() + peer.close() + + +@pytest.mark.parametrize("failure", [ConnectionResetError(), http.client.IncompleteRead(b"synthetic-private")]) +def test_broken_error_body_keeps_status_and_never_echoes_partial_content(failure): + class Broken(Response): + def read1(self, n=-1): + raise failure + + stream = Broken() + error = urllib.error.HTTPError(CREDS["endpoint"], 403, "denied", {}, stream) + with patch.object(dh._opener, "open", side_effect=error): + assert dh.http_json("GET", CREDS["endpoint"]) == ( + 403, {"error": "error body could not be read"}) + assert stream.closed + + +@pytest.mark.parametrize("tid,valid", [("1", True), ("abc123", True), ("a" * 16, True), + ("B" * 32, True), ("", False), ("a" * 33, False), + ("xyz", False), ("1\n", False)]) +def test_jaeger_search_accepts_unpadded_ids_without_accepting_malformed_ids(tid, valid): + module = importlib.import_module("jaeger_mcp") + trace = {"traceID": tid, "spans": [{"startTime": 1, "duration": 1, "operationName": "op"}], "processes": {}} + with patch.object(module, "_ds", return_value=CREDS), patch.object(module, "_get", return_value={"data": [trace]}): + result = json.loads(module.jaeger_search({"query": "svc"})["body"]) + assert result["collectionStatus"] == ("ok" if valid else "unknown") + assert result["returnedCount"] == int(valid) + if valid: + assert result["traces"][0]["traceID"] == tid + + +def test_loki_metric_series_do_not_saturate_the_log_line_limit(): + module = importlib.import_module("loki_mcp") + data = {"resultType": "vector", "result": [{"metric": {}, "value": [1, "2"]}]} + result = json.loads(module._query_response(data, requested=1)["body"]) + assert result["collectionStatus"] == "ok" + assert result["limitSaturated"] is False + + +def test_loki_returned_count_describes_retained_lines_after_local_bound(): + module = importlib.import_module("loki_mcp") + data = {"resultType": "streams", "result": [{"stream": {}, "values": [["1", "a"], ["2", "b"]]}]} + with patch.object(module, "MAX_TOTAL_LINES", 1): + result = json.loads(module._query_response(data, requested=10)["body"]) + assert result["truncated"] is True + assert result["returnedCount"] == len(result["result"][0]["values"]) == 1 + + def test_loki_limit_saturation_is_disclosed_as_partial(): module = importlib.import_module("loki_mcp") hit = {"status": "success", "data": {"resultType": "streams", "result": [{"stream": {"a": "b"}, "values": [["1", "line"]]}]}} diff --git a/docs/runbooks/AGENTS.md b/docs/runbooks/AGENTS.md index c6e54020f..91f548e9d 100644 --- a/docs/runbooks/AGENTS.md +++ b/docs/runbooks/AGENTS.md @@ -1,4 +1,4 @@ - + > You are an external reviewer for this repo — project context below, distilled from CLAUDE.md. This file is shared verbatim by Kiro, Codex, and Agy (not a per-AI copy). @@ -13,6 +13,8 @@ Web image receipt/ECR proof precedes private migrations, including reuse; promotion retains that digest. Automatic DDL is expand-only; contract cutovers require a merge freeze, drained queues and explicit operator coordination. Legacy images without receipts use the separately approved `legacy-web-image-recovery.md` path with trusted digest/source evidence and schema approval, never a fabricated receipt or mutable-tag fallback. Operational playbooks organized by scenario, each following symptoms → diagnosis → action. +`datasource-credential-isolation.md` requires native-default credential backfill before stricter +explicit-ID reads roll out; missing or foreign credentials fail closed (ADR-007/039). Graph collection/projection, freshness and retained-evidence rollout contracts live in `source-sync-observability.md`. See `docs/runbooks/CLAUDE.md`'s index for the current runbook list (several are marked **v1 (legacy)** — v2 has since replaced their procedure with a different mechanism; don't treat a diff --git a/docs/runbooks/CLAUDE.md b/docs/runbooks/CLAUDE.md index 2012899a7..279e05191 100644 --- a/docs/runbooks/CLAUDE.md +++ b/docs/runbooks/CLAUDE.md @@ -24,6 +24,7 @@ erasing valid Read evidence. Source helper/tests and the role consumer catalog a | [k8sgpt-operator-install.md](k8sgpt-operator-install.md) | Out-of-band K8sGPT operator install (manual operator work, ADR-005 precedent) | | [alert-pipeline-troubleshoot.md](alert-pipeline-troubleshoot.md) | Alert pipeline failure response (ADR-008/013) | | [cache-warmer-issues.md](cache-warmer-issues.md) | Cache warmer staleness / error response | +| [datasource-credential-isolation.md](datasource-credential-isolation.md) | Native datasource credential identity and mandatory default backfill before rollout (ADR-007/039) | | [tempo-query-generation.md](tempo-query-generation.md) | Tempo query generation — connector/web deployment, admin API schema refresh, cached summaries, and recent-window limits | | [graph-read-contract.md](graph-read-contract.md) | Bounded graph reads/rebuilds, shared admission, infra dependency, publication outcomes, registry diagnostics and PostgreSQL fixtures | | [source-sync-observability.md](source-sync-observability.md) | Public source rollout — flow/infra/trace collection clocks, retention/budgets, reader projections, DX scope and deployment prerequisites | diff --git a/docs/runbooks/datasource-credential-isolation.md b/docs/runbooks/datasource-credential-isolation.md index f1548d6af..0114fee6d 100644 --- a/docs/runbooks/datasource-credential-isolation.md +++ b/docs/runbooks/datasource-credential-isolation.md @@ -1,5 +1,15 @@ # Datasource credential identity and rollout +This runbook covers native datasource isolation under ADR-007 external-data governance +and ADR-039 integration identity. It does not approve deployment or activate any gate. + +## Symptoms + +After a stricter connector rollout, a migrated default with only an unstamped kind +credential fails an explicit-ID read. A sibling without its own credentials must fail. + +## Diagnosis + Native datasource connectors store one credential blob per integration ID. Their plain kind key is a managed mirror for the current default. The mirror includes `instanceId`; an explicit-ID producer read accepts only its own ID key @@ -7,6 +17,8 @@ or a mirror bearing that exact ID. Missing credentials fail closed. Calls withou an explicit ID retain the existing kind-default lookup. An old unstamped mirror cannot prove which instance owns its credentials. +## Action + Before deploying the stricter connector producer, operators must backfill every migrated default through the existing authenticated administrator save flow: diff --git a/web/app/api/datasources/manage/route.test.ts b/web/app/api/datasources/manage/route.test.ts index f69dddac8..0f75a7df2 100644 --- a/web/app/api/datasources/manage/route.test.ts +++ b/web/app/api/datasources/manage/route.test.ts @@ -124,6 +124,16 @@ describe('POST create', () => { }); }); +it('backfills a stamped mirror without persisting its identity stamp in the ID credential', async () => { + getDatasource.mockResolvedValue({ id: 7, kind: 'prometheus', endpoint: 'http://old:9090', authType: 'basic', isDefault: true, settings: {} }); + getCredentialById.mockResolvedValue({ endpoint: 'http://old:9090', authType: 'basic', username: 'u', password: 'pw', instanceId: '7' }); + const { PATCH } = await import('./route'); + const res = await PATCH(req({ id: 7, settings: { timeoutS: 15 } }, 'PATCH')); + expect(res.status).toBe(200); + expect(setIntegrationCredentialById.mock.calls.at(-1)![1]).toMatchObject({ username: 'u', password: 'pw' }); + expect(setIntegrationCredentialById.mock.calls.at(-1)![1].instanceId).toBeUndefined(); +}); + describe('PATCH update', () => { it('404 when not found', async () => { getDatasource.mockResolvedValue(null); diff --git a/web/app/api/datasources/manage/route.ts b/web/app/api/datasources/manage/route.ts index dadcec332..8dc33c4e0 100644 --- a/web/app/api/datasources/manage/route.ts +++ b/web/app/api/datasources/manage/route.ts @@ -184,6 +184,7 @@ export async function PATCH(request: Request) { // PATCH must not carry a historical stale timeoutS/database forward either). delete existing.timeoutS; delete existing.database; + delete existing.instanceId; // ORIGIN compare (scheme+host+port — an https→http downgrade must count as a change, or // Basic material would transmit in cleartext); a malformed URL counts as changed. const originOf = (u: string | null | undefined): string | null => { try { return new URL(u ?? '').origin; } catch { return null; } }; diff --git a/web/lib/datasources.ts b/web/lib/datasources.ts index 24eb318e3..a5c43bb9d 100644 --- a/web/lib/datasources.ts +++ b/web/lib/datasources.ts @@ -186,7 +186,7 @@ export async function resolveConnConfig(ds: DatasourceRow): Promise ...(ds.authType ? { authType: ds.authType } : {}), // gap L203: settings ride the conn config so the Explore route, the service-graph sources and // the credential-blind agent/worker path (which reads the same keys from the secret blob) all get - // ONE bound from one mechanism. database (identifier-validated on write AND read) is + // Saved provider ceiling; Explore may tighten it further to 10s. database (identifier-validated on write AND read) is // ClickHouse-only; timeoutS goes to every kind — Prometheus/Mimir/ClickHouse apply it as the // provider execution ceiling, the other kinds as the transport deadline. ...(ds.kind === 'clickhouse' && ds.settings?.database ? { database: ds.settings.database } : {}), From 1e6e3a86149bae822375694bee52c1bac1899aa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=98=A4=EC=A4=80=EC=84=9D=28Junseok=20Oh=29DevOps?= <52226147+Atom-oh@users.noreply.github.com> Date: Mon, 19 Oct 2026 16:13:58 +0900 Subject: [PATCH 06/27] fix(agent): pin custom MCP connections to the validated address (#184) Custom MCP transports connect to the address set validated at request time while preserving Host, TLS SNI and certificate verification, and reject every 3xx. IPv6 addresses are bracketed so AAAA-only endpoints connect. The transport deadline and the MCP session read timeout follow a 60s read budget separate from the 8s connect budget. A malformed port is an SSRF-classified rejection, discarded per-address failures are logged, and pasted trailing whitespace in an api_key value is stripped. The pin composes with the default-off custom MCP governance: spec validation and the attested secret version run first, then the validated addresses are pinned for that session. Co-authored-by: Claude Fable 5.1 --- agent/Dockerfile | 4 +- agent/agent.py | 66 +++++++++-- agent/lambda/datasource_http.py | 6 +- agent/pinned_http.py | 86 ++++++++++++++ agent/requirements.txt | 3 + agent/streamable_http_sigv4.py | 7 +- agent/test_agent.py | 50 ++++++++ agent/test_pinned_http.py | 194 ++++++++++++++++++++++++++++++++ 8 files changed, 398 insertions(+), 18 deletions(-) create mode 100644 agent/pinned_http.py create mode 100644 agent/test_pinned_http.py diff --git a/agent/Dockerfile b/agent/Dockerfile index db5f2e523..dd7942094 100644 --- a/agent/Dockerfile +++ b/agent/Dockerfile @@ -18,11 +18,13 @@ RUN pip install --no-cache-dir \ # CLICKHOUSE_* env contract, or the exposed tool surface with no review signal. # Keep in sync with agent/requirements.txt. "mcp-clickhouse==0.4.1" \ - httpx \ + "httpx==0.28.1" \ + "httpcore==1.0.9" \ httpx-sse \ aws-requests-auth COPY streamable_http_sigv4.py . +COPY pinned_http.py . COPY account_utils.py . COPY agent.py . COPY readiness.py . diff --git a/agent/agent.py b/agent/agent.py index 8b1334b89..8e8cbd314 100644 --- a/agent/agent.py +++ b/agent/agent.py @@ -22,6 +22,7 @@ from strands.tools.mcp.mcp_client import MCPClient from botocore.credentials import Credentials from bedrock_agentcore.runtime import BedrockAgentCoreApp +import pinned_http from streamable_http_sigv4 import streamablehttp_client_with_sigv4, streamablehttp_client_with_headers from account_utils import effective_account_id import boto3 @@ -520,11 +521,31 @@ class SsrfBlocked(Exception): }) +_NAT64_PREFIXES = (ipaddress.ip_network("64:ff9b::/96"), ipaddress.ip_network("64:ff9b:1::/48")) + + +def _embedded_ipv4(ip): + """Classify an IPv6 address by the IPv4 it embeds, so a translator cannot launder a blocked + target: IPv4-mapped (::ffff:a.b.c.d), 6to4 (2002::/16), Teredo (2001::/32, client address) and + NAT64 (64:ff9b::/96, 64:ff9b:1::/48 — low 32 bits). Plain addresses are returned unchanged.""" + if ip.version != 6: + return ip + mapped = ip.ipv4_mapped or ip.sixtofour + if mapped is not None: + return mapped + teredo = ip.teredo + if teredo is not None: + return teredo[1] + if any(ip in net for net in _NAT64_PREFIXES): + return ipaddress.ip_address(int(ip) & 0xFFFFFFFF) + return ip + + def _ip_always_blocked(ip_str): """Returns True if the IP is metadata, loopback, link-local, multicast, reserved, or unspecified — i.e. blocked REGARDLESS of the per-account allowPrivate opt-in.""" try: - ip = ipaddress.ip_address(ip_str) + ip = _embedded_ipv4(ipaddress.ip_address(ip_str)) if ip in _METADATA_IPS: return True return (ip.is_loopback or ip.is_link_local or ip.is_multicast or @@ -536,7 +557,7 @@ def _ip_always_blocked(ip_str): def _ip_is_private(ip_str): """Returns True if the IP is in a private range (RFC1918/ULA) AND not always blocked.""" try: - ip = ipaddress.ip_address(ip_str) + ip = _embedded_ipv4(ipaddress.ip_address(ip_str)) # ipaddress.is_private includes link-local/loopback in some versions; we only want # the "opt-in" ranges (RFC1918 + RFC4193 ULA). return ip.is_private and not _ip_always_blocked(ip_str) @@ -548,11 +569,10 @@ def _assert_host_allowed(url, allow_private, resolver=socket.getaddrinfo): """Assert the URL host is connectable: require https, resolve the host, and block any resolved IP that is always-blocked (metadata/loopback/link-local/...) or private-without-the-opt-in. - KNOWN LIMITATION (P4 gate, accepted — matches ADR-011 / web/lib/ssrf-guard.ts, both resolve-based): - this is resolve-and-recheck; the transport re-resolves at connect time, so a DNS-rebinding host - could return a safe IP here and a blocked IP at connect. The layered mitigations are redirect:'manual' - (set on both integration transports) + the registration-time literal-host guard (ssrf-guard.ts). True - IP-pinning (connect to the validated IP with SNI/Host preserved) is deferred to P3 hardening.""" + Return all validated IPs for the integration transport, which pins connections while + preserving Host, TLS SNI and certificate verification.""" + if "\\" in str(url): + raise SsrfBlocked("SSRF block: endpoint must not contain a backslash") parsed = urllib.parse.urlparse(url) if parsed.scheme != 'https': raise SsrfBlocked(f"SSRF block: HTTPS required for integration endpoints, got {parsed.scheme}") @@ -561,21 +581,29 @@ def _assert_host_allowed(url, allow_private, resolver=socket.getaddrinfo): if not host: raise SsrfBlocked(f"SSRF block: invalid or missing host in URL {url}") + try: + port = parsed.port or 443 # ValueError on a malformed port such as https://host:abc/ + except ValueError: + raise SsrfBlocked(f"SSRF block: invalid port in URL {url}") try: # Resolve all IPs for the host (could be dual-stack or have multiple A records) - addr_info = resolver(host, 443, proto=socket.IPPROTO_TCP) + addr_info = resolver(host, port, proto=socket.IPPROTO_TCP) except socket.gaierror as e: raise SsrfBlocked(f"SSRF block: could not resolve host {host}: {e}") if not addr_info: raise SsrfBlocked(f"SSRF block: could not resolve host {host} (no addresses returned)") + addresses = [] for family, type, proto, canonname, sockaddr in addr_info: ip_str = sockaddr[0] if _ip_always_blocked(ip_str): raise SsrfBlocked(f"SSRF block: host {host} resolved to always-blocked IP {ip_str}") if _ip_is_private(ip_str) and not allow_private: raise SsrfBlocked(f"SSRF block: host {host} resolved to private IP {ip_str} and private access disabled") + if ip_str not in addresses: + addresses.append(ip_str) + return tuple(addresses) def parse_secret(val): @@ -599,12 +627,22 @@ def auth_headers(transport, secret): val = secret.get("value", secret.get("api_key", secret.get("_raw"))) if not val: raise ValueError(f"Integration [api_key] missing value in secret") + # `authorization` is deliberately allowed here (unlike datasource_http's list): an api_key + # integration may legitimately carry its key in the Authorization header. + val = val.strip() if isinstance(val, str) else val # a pasted trailing newline is not a secret + if (not isinstance(header, str) or not re.fullmatch(r"[A-Za-z0-9!#$%&'*+.^_`|~-]+", header) + or header.lower() in ("host", "content-length", "transfer-encoding", "connection")): + raise ValueError("Integration [api_key] header name is invalid") + if not isinstance(val, str) or not val or any(ord(ch) < 32 or ord(ch) > 126 for ch in val): + raise ValueError("Integration [api_key] header value contains unsupported characters") return {header: val} if transport == 'oauth_client_credentials': # Bearer-style for this increment: {"token": "..."} or raw val = secret.get("token", secret.get("_raw")) if not val: raise ValueError(f"Integration [oauth_client_credentials] missing token in secret") + if not isinstance(val, str) or any(ord(ch) < 32 or ord(ch) > 126 for ch in val): + raise ValueError("Integration stored bearer token is invalid") return {"Authorization": f"Bearer {val}"} if transport == 'sigv4': return {} @@ -639,7 +677,8 @@ def sigv4_params(endpoint, service=None, region=None): # Short per-integration connect/read timeout — chat maxDuration is 120s; a slow or hung external # MCP must not dominate the budget (the default 30s transport timeout is too long for N integrations). -INTEGRATION_CONNECT_TIMEOUT = 8 +INTEGRATION_CONNECT_TIMEOUT = pinned_http.CONNECT_TIMEOUT # TCP+TLS per pinned address +INTEGRATION_READ_TIMEOUT = pinned_http.READ_TIMEOUT # time to headers / between SSE events _secrets_client = None @@ -764,12 +803,15 @@ def _connect_integration(spec, stack, account_context=None): transport = spec.get("transport") allow_private = bool(spec.get("allowPrivate")) exposed = spec.get("exposedTools") or [] - # Connection-time SSRF (https + DNS resolve-and-recheck + private opt-in) BEFORE any network call. - _assert_host_allowed(endpoint, allow_private) + # Connection-time SSRF (https + private opt-in) BEFORE any network call. The validated address + # set is invocation-local: resolved once here and pinned for this MCP session only (it may outlive + # the DNS TTL by design — the pin is the point); warm invocations never share DNS pins. + pinned_ips = _assert_host_allowed(endpoint, allow_private) secret = parse_secret(_get_secret(spec["credentialsRef"], expected_version=version)) headers = auth_headers(transport, secret) client = MCPClient(lambda: streamablehttp_client_with_headers( - url=endpoint, headers=headers, timeout=INTEGRATION_CONNECT_TIMEOUT)) + url=endpoint, headers=headers, timeout=INTEGRATION_CONNECT_TIMEOUT, + sse_read_timeout=INTEGRATION_READ_TIMEOUT, pinned_ips=pinned_ips)) stack.enter_context(client) live = get_all_tools(client) selected = select_integration_tools(live, exposed) diff --git a/agent/lambda/datasource_http.py b/agent/lambda/datasource_http.py index c4522db85..a7a53a8d2 100644 --- a/agent/lambda/datasource_http.py +++ b/agent/lambda/datasource_http.py @@ -27,9 +27,9 @@ calls assert_host_allowed(endpoint) immediately before http_json(same endpoint) — that invariant is what makes the cache safe; it is reset every invocation by set_request_conn() (called at the top of every lambda_handler) so a warm container never reuses a stale pinned IP across invocations. -(A structurally identical resolve-and-recheck gap exists in agent/agent.py's `_assert_host_allowed` -for the separate egress-integrations transport — untouched here; that's a different call path not -exercised by the pentest's datasource/schema-fetch findings, and needs its own fix.) +The separate custom-MCP transport (agent/agent.py `_assert_host_allowed` + agent/pinned_http.py) pins +its invocation-local validated address set the same way; registration-time checks (web/lib/ssrf-guard.ts) +stay resolve-based, connection time is pinned on both transports. """ import base64 import functools diff --git a/agent/pinned_http.py b/agent/pinned_http.py new file mode 100644 index 000000000..f0ea05312 --- /dev/null +++ b/agent/pinned_http.py @@ -0,0 +1,86 @@ +"""HTTPX transport for one administrator-approved MCP origin and validated IP set.""" +import ipaddress +import asyncio +import time + +import httpx + +# Connect budget (TCP + TLS to one pinned address) and the separate read budget (time to response +# headers / between SSE events). The read budget stays well under the 120s chat request budget +# but must not collapse to the connect budget: an integration whose upstream takes a few seconds to +# start answering is a normal tool call, not a dead connection. +CONNECT_TIMEOUT = 8 +READ_TIMEOUT = 60 + + +def _pinned_host(address): + """URL host form of a validated address: IPv6 literals must be bracketed or httpx treats the + colon-bearing string as a registered name and the pin fails for every AAAA-only endpoint.""" + return f"[{address}]" if ":" in address else address + + +class PinnedAsyncTransport(httpx.AsyncBaseTransport): + def __init__(self, endpoint, addresses, transport=None): + self.endpoint = httpx.URL(endpoint) + self.addresses = tuple(str(ipaddress.ip_address(address)) for address in addresses) + if self.endpoint.scheme != "https" or not self.addresses: + raise ValueError("MCP HTTPS origin and validated addresses are required") + self.transport = transport or httpx.AsyncHTTPTransport(trust_env=False) + + async def handle_async_request(self, request): + origin = lambda url: (url.scheme, url.host, url.port or 443) + if origin(request.url) != origin(self.endpoint): + raise httpx.ConnectError("MCP request origin was not approved", request=request) + # The transport-level deadline bounds the time to response headers (connect + request + + # first response bytes): it is a READ budget, so it follows the request's read timeout and + # never the much shorter connect timeout. + timeouts = request.extensions.get("timeout") or {} + budget = timeouts.get("read") + budget = budget if isinstance(budget, (int, float)) and budget > 0 else READ_TIMEOUT + deadline = time.monotonic() + budget + last_error = None + for address in self.addresses: + # Auth runs against the original URL. Only the socket destination changes. + pinned = httpx.Request( + request.method, request.url.copy_with(host=_pinned_host(address)), + headers=request.headers, stream=request.stream, + extensions={**request.extensions, "sni_hostname": request.url.raw_host.decode("ascii")}, + ) + try: + remaining = deadline - time.monotonic() + if remaining <= 0: + raise httpx.ReadTimeout("MCP connection request deadline exceeded", request=request) + async with asyncio.timeout(remaining): + response = await self.transport.handle_async_request(pinned) + except TimeoutError as exc: + # The timeout may occur after body delivery, so it must not be replayed. + raise httpx.ReadTimeout("MCP connection request deadline exceeded", request=request) from exc + except (httpx.ConnectError, httpx.ConnectTimeout) as exc: + # Connection failure occurs before a body is sent; never replay read/write failures. + # Each discarded attempt is logged so a certificate-verification failure on one + # address is not hidden behind the last address's error. + print(f"[pinned_http] {address} failed before any body was sent: {type(exc).__name__}: {exc}") + last_error = exc + continue + if 300 <= response.status_code < 400: + await response.aclose() + # 3xx without Location (e.g. 304) is still outside the MCP contract; the message is + # about the class of response, so it stays accurate for both. + raise httpx.HTTPStatusError("MCP 3xx response rejected (redirects are never followed)", + request=request, response=response) + return response + raise last_error + + async def aclose(self): + await self.transport.aclose() + + +def pinned_client_factory(endpoint, addresses): + """McpHttpClientFactory contract; no proxy, redirects, or second hostname lookup.""" + def factory(headers=None, timeout=None, auth=None): + return httpx.AsyncClient( + headers=headers, timeout=timeout or httpx.Timeout(READ_TIMEOUT, connect=CONNECT_TIMEOUT), + auth=auth, follow_redirects=False, max_redirects=0, trust_env=False, + transport=PinnedAsyncTransport(endpoint, addresses), + ) + return factory diff --git a/agent/requirements.txt b/agent/requirements.txt index e255fb8a5..a0964e06e 100644 --- a/agent/requirements.txt +++ b/agent/requirements.txt @@ -3,6 +3,9 @@ strands-agents==1.41.0 strands-agents-tools boto3 +# Custom MCP pinning uses the tested HTTPX/HTTPcore SNI extension contract. +httpx==0.28.1 +httpcore==1.0.9 # Dark-path chat loop (ANTHROPIC_AGENT_LOOP_ENABLED, default OFF) — Bedrock client, no API key. # Keep the pin in sync with agent/Dockerfile. Validate at `make agentcore` build/smoke. anthropic[bedrock]==0.111.0 diff --git a/agent/streamable_http_sigv4.py b/agent/streamable_http_sigv4.py index 83309d1d3..57017a494 100644 --- a/agent/streamable_http_sigv4.py +++ b/agent/streamable_http_sigv4.py @@ -11,6 +11,7 @@ from typing import Generator import httpx +from pinned_http import pinned_client_factory from anyio.streams.memory import MemoryObjectReceiveStream, MemoryObjectSendStream from botocore.auth import SigV4Auth from botocore.awsrequest import AWSRequest @@ -132,6 +133,7 @@ async def streamablehttp_client_with_sigv4( sse_read_timeout: float | timedelta = 60 * 5, terminate_on_close: bool = True, httpx_client_factory: McpHttpClientFactory = create_mcp_http_client, + pinned_ips: tuple[str, ...] | None = None, ) -> AsyncGenerator[ tuple[ MemoryObjectReceiveStream[SessionMessage | Exception], @@ -149,7 +151,7 @@ async def streamablehttp_client_with_sigv4( timeout=timeout, sse_read_timeout=sse_read_timeout, terminate_on_close=terminate_on_close, - httpx_client_factory=_no_redirect_http_client_factory, + httpx_client_factory=pinned_client_factory(url, pinned_ips) if pinned_ips is not None else _no_redirect_http_client_factory, auth=SigV4HTTPXAuth(credentials, service, region), ) as result: yield result @@ -162,6 +164,7 @@ async def streamablehttp_client_with_headers( timeout: float | timedelta = 30, sse_read_timeout: float | timedelta = 60 * 5, terminate_on_close: bool = True, + pinned_ips: tuple[str, ...] | None = None, ) -> AsyncGenerator[ tuple[ MemoryObjectReceiveStream[SessionMessage | Exception], @@ -179,6 +182,6 @@ async def streamablehttp_client_with_headers( timeout=timeout, sse_read_timeout=sse_read_timeout, terminate_on_close=terminate_on_close, - httpx_client_factory=_no_redirect_http_client_factory, + httpx_client_factory=pinned_client_factory(url, pinned_ips) if pinned_ips is not None else _no_redirect_http_client_factory, ) as result: yield result diff --git a/agent/test_agent.py b/agent/test_agent.py index faaf7f16f..98facaea8 100644 --- a/agent/test_agent.py +++ b/agent/test_agent.py @@ -69,6 +69,36 @@ def __init__(self, name): self.tool_name = name +class IntegrationCredentialValidationTest(unittest.TestCase): + def test_transport_headers_and_control_characters_fail_without_echoing_secrets(self): + cases = [ + ("api_key", {"header": "Host", "value": "synthetic-secret"}), + ("api_key", {"header": "Content-Length", "value": "synthetic-secret"}), + ("api_key", {"header": "X-Key", "value": "synthetic-secret\nvalue"}), + ("oauth_client_credentials", {"token": "synthetic-secret\nvalue"}), + ("api_key", {"header": "X-Key", "value": "synthetic-secret\u200b"}), + ] + for transport, secret in cases: + with self.subTest(transport=transport, header=secret.get("header")): + with self.assertRaises(ValueError) as error: + agent.auth_headers(transport, secret) + self.assertNotIn("synthetic-secret", str(error.exception)) + + def test_pasted_trailing_whitespace_is_stripped_not_rejected(self): + # A copied token often ends in a newline; that is not a control character inside the secret. + self.assertEqual(agent.auth_headers("api_key", {"header": "X-Key", "value": "synthetic-secret\n"}), + {"X-Key": "synthetic-secret"}) + with self.assertRaises(ValueError) as error: + agent.auth_headers("api_key", {"header": "X-Key", "value": " \n"}) # whitespace only = missing + self.assertNotIn("synthetic", str(error.exception)) + + def test_supported_authorization_headers_are_preserved(self): + self.assertEqual(agent.auth_headers("api_key", {"value": "Bearer synthetic"}), + {"Authorization": "Bearer synthetic"}) + self.assertEqual(agent.auth_headers("oauth_client_credentials", {"token": "synthetic"}), + {"Authorization": "Bearer synthetic"}) + + def names(tools): return [t.tool_name for t in tools] @@ -232,6 +262,26 @@ def resolver(host, port, *a, **k): with self.assertRaisesRegex(agent.SsrfBlocked, "could not resolve"): agent._assert_host_allowed("https://nxdomain.local", True, resolver=resolver) + # Translator-embedded forms of the metadata/loopback targets are classified by the IPv4 they + # carry: IPv4-mapped, 6to4, Teredo (client address) and NAT64 (well-known and local-use). + for embedded in ("::ffff:169.254.169.254", "2002:a9fe:a9fe::1", "2001::5601:5601", + "64:ff9b::a9fe:a9fe", "64:ff9b:1::a9fe:a9fe", "64:ff9b::7f00:1"): + self.assertTrue(agent._ip_always_blocked(embedded), embedded) + self.assertFalse(agent._ip_is_private(embedded), embedded) + self.assertTrue(agent._ip_is_private("64:ff9b::a00:1")) # NAT64 of 10.0.0.1 = private (opt-in) + self.assertFalse(agent._ip_always_blocked("64:ff9b::0808:0808")) # NAT64 of 8.8.8.8 = public + + # A malformed port is an SSRF-classified rejection, not an uncaught ValueError. + with self.assertRaisesRegex(agent.SsrfBlocked, "invalid port"): + agent._assert_host_allowed("https://public.com:abc/", False, resolver=resolver) + + def test_integration_session_gets_separate_connect_and_read_budgets(self): + # Collapsing the read budget to the 8s connect budget made every tool whose upstream takes + # longer than 8s to start answering fail with ReadTimeout. + self.assertEqual(agent.INTEGRATION_CONNECT_TIMEOUT, 8) + self.assertGreater(agent.INTEGRATION_READ_TIMEOUT, agent.INTEGRATION_CONNECT_TIMEOUT) + self.assertLessEqual(agent.INTEGRATION_READ_TIMEOUT, 120) # inside the chat request budget + class IntegrationHelpersTest(unittest.TestCase): def test_parse_secret(self): diff --git a/agent/test_pinned_http.py b/agent/test_pinned_http.py new file mode 100644 index 000000000..7fe248635 --- /dev/null +++ b/agent/test_pinned_http.py @@ -0,0 +1,194 @@ +"""Custom MCP pinning: origin, redirects, replay boundaries and real TLS verification.""" +import asyncio +import http.server +import ssl +import subprocess +import sys +import threading +from pathlib import Path + +import httpx +import pytest + +sys.path.insert(0, str(Path(__file__).parent)) +from pinned_http import PinnedAsyncTransport, pinned_client_factory + +ORIGIN = "https://synthetic-mcp.example.test:8443/mcp" + + +def test_pins_socket_target_and_preserves_host_sni_body_and_auth_origin(): + seen = {} + class Auth(httpx.Auth): + def auth_flow(self, request): + seen["auth_url"] = str(request.url) + request.headers["Authorization"] = "synthetic" + yield request + async def upstream(request): + seen.update(url=str(request.url), host=request.headers["Host"], + sni=request.extensions["sni_hostname"], + body=await request.aread(), authorization=request.headers["Authorization"]) + return httpx.Response(200, json={"ok": True}) + async def run(): + transport = PinnedAsyncTransport(ORIGIN, ("192.0.2.10",), transport=httpx.MockTransport(upstream)) + async with httpx.AsyncClient(transport=transport, auth=Auth()) as client: + response = await client.post(ORIGIN, content=b"synthetic body") + assert response.json()["ok"] + asyncio.run(run()) + assert seen == { + "auth_url": ORIGIN, "url": "https://192.0.2.10:8443/mcp", + "host": "synthetic-mcp.example.test:8443", "sni": "synthetic-mcp.example.test", + "body": b"synthetic body", "authorization": "synthetic", + } + + +def test_ipv6_pinned_address_is_bracketed_so_aaaa_only_endpoints_work(): + seen = {} + async def upstream(request): + seen.update(url=str(request.url), host=request.headers["Host"], sni=request.extensions["sni_hostname"]) + return httpx.Response(200, json={"ok": True}) + async def run(): + transport = PinnedAsyncTransport(ORIGIN, ("2001:db8::10",), transport=httpx.MockTransport(upstream)) + async with httpx.AsyncClient(transport=transport) as client: + assert (await client.post(ORIGIN, content=b"x")).json()["ok"] + asyncio.run(run()) + assert seen == {"url": "https://[2001:db8::10]:8443/mcp", "host": "synthetic-mcp.example.test:8443", + "sni": "synthetic-mcp.example.test"} + + +def test_transport_deadline_follows_the_read_budget_not_the_connect_budget(): + # A tool whose upstream takes longer than the connect timeout to START answering is a normal + # call; only the read budget may cut it off. + async def upstream(request): + await asyncio.sleep(0.05) + return httpx.Response(200, json={"ok": True}) + async def run(): + transport = PinnedAsyncTransport(ORIGIN, ("192.0.2.10",), transport=httpx.MockTransport(upstream)) + async with httpx.AsyncClient(transport=transport, timeout=httpx.Timeout(5, connect=0.001)) as client: + assert (await client.post(ORIGIN, content=b"x")).json()["ok"] + async with httpx.AsyncClient(transport=transport, timeout=httpx.Timeout(0.001, connect=5)) as client: + with pytest.raises(httpx.ReadTimeout): + await client.post(ORIGIN, content=b"x") + asyncio.run(run()) + + +def test_factory_default_timeout_separates_connect_and_read_budgets(): + import pinned_http + async def run(): + async with pinned_client_factory(ORIGIN, ("192.0.2.10",))() as client: + assert client.timeout.connect == pinned_http.CONNECT_TIMEOUT + assert client.timeout.read == pinned_http.READ_TIMEOUT > pinned_http.CONNECT_TIMEOUT + asyncio.run(run()) + + +@pytest.mark.parametrize("location", ["/next", "https://169.254.169.254/", "https://another.example.test/mcp"]) +def test_rejects_all_redirects_before_mcp_sdk_can_follow_them(location): + calls = [] + async def upstream(request): + calls.append(request) + return httpx.Response(307, headers={"Location": location}) + async def run(): + transport = PinnedAsyncTransport(ORIGIN, ("192.0.2.10",), transport=httpx.MockTransport(upstream)) + async with httpx.AsyncClient(transport=transport, follow_redirects=True) as client: + with pytest.raises(httpx.HTTPStatusError, match="3xx response rejected"): + await client.post(ORIGIN, content=b"synthetic") + asyncio.run(run()) + assert len(calls) == 1 + + +@pytest.mark.parametrize("changed", [ + "https://another.example.test:8443/mcp", "https://synthetic-mcp.example.test/mcp", + "http://synthetic-mcp.example.test:8443/mcp", +]) +def test_unapproved_origin_never_reaches_transport(changed): + async def upstream(request): + pytest.fail("unapproved origin reached transport") + async def run(): + transport = PinnedAsyncTransport(ORIGIN, ("192.0.2.10",), transport=httpx.MockTransport(upstream)) + async with httpx.AsyncClient(transport=transport) as client: + with pytest.raises(httpx.ConnectError, match="origin was not approved"): + await client.get(changed) + asyncio.run(run()) + + +@pytest.mark.parametrize("error,expected_calls", [(httpx.ConnectError, 2), (httpx.ReadError, 1), (httpx.WriteError, 1)]) +def test_fallback_is_only_before_body_delivery(error, expected_calls): + calls = [] + async def upstream(request): + calls.append(request.url.host) + raise error("synthetic failure", request=request) + async def run(): + transport = PinnedAsyncTransport(ORIGIN, ("192.0.2.10", "192.0.2.11"), transport=httpx.MockTransport(upstream)) + async with httpx.AsyncClient(transport=transport) as client: + with pytest.raises(error): + await client.post(ORIGIN, content=b"synthetic") + asyncio.run(run()) + assert len(calls) == expected_calls + + +def test_factory_disables_proxy_routing_and_sdk_redirect_budget(): + async def run(): + async with pinned_client_factory(ORIGIN, ("192.0.2.10",))() as client: + assert client.follow_redirects is False and client.max_redirects == 0 + assert isinstance(client._transport, PinnedAsyncTransport) + asyncio.run(run()) + + +@pytest.fixture +def tls_server(tmp_path): + host = "mcp-never-resolved.invalid" + certificate, key = tmp_path / "cert.pem", tmp_path / "key.pem" + subprocess.run([ + "openssl", "req", "-x509", "-newkey", "rsa:2048", "-nodes", "-keyout", str(key), + "-out", str(certificate), "-days", "1", "-subj", f"/CN={host}", + "-addext", f"subjectAltName=DNS:{host}", + ], check=True, capture_output=True, timeout=30) + seen = {} + class Handler(http.server.BaseHTTPRequestHandler): + def do_GET(self): + seen["host"] = self.headers["Host"] + payload = b'{"ok":true}' + self.send_response(200) + self.send_header("Content-Length", str(len(payload))) + self.end_headers() + self.wfile.write(payload) + def log_message(self, *args): + pass + server = http.server.HTTPServer(("127.0.0.1", 0), Handler) + context = ssl.SSLContext(ssl.PROTOCOL_TLS_SERVER) + context.minimum_version = ssl.TLSVersion.TLSv1_2 + context.load_cert_chain(str(certificate), str(key)) + context.set_servername_callback(lambda sock, name, ctx: seen.update(sni=name)) + server.socket = context.wrap_socket(server.socket, server_side=True) + thread = threading.Thread(target=server.serve_forever, daemon=True) + thread.start() + try: + yield host, server.server_port, ssl.create_default_context(cafile=str(certificate)), seen + finally: + server.shutdown() + server.server_close() + thread.join(timeout=2) + + +def test_real_tls_uses_original_hostname_with_a_pinned_socket(tls_server): + host, port, context, seen = tls_server + async def run(): + transport = PinnedAsyncTransport(f"https://{host}:{port}/", ("127.0.0.1",), + transport=httpx.AsyncHTTPTransport(verify=context, trust_env=False)) + async with httpx.AsyncClient(transport=transport) as client: + response = await client.get(f"https://{host}:{port}/probe") + assert response.json() == {"ok": True} + asyncio.run(run()) + assert seen == {"sni": host, "host": f"{host}:{port}"} + + +def test_real_tls_still_rejects_a_wrong_certificate_hostname(tls_server): + host, port, context, seen = tls_server + async def run(): + endpoint = f"https://wrong-certificate.invalid:{port}/" + transport = PinnedAsyncTransport(endpoint, ("127.0.0.1",), + transport=httpx.AsyncHTTPTransport(verify=context, trust_env=False)) + async with httpx.AsyncClient(transport=transport) as client: + with pytest.raises(httpx.ConnectError): + await client.get(endpoint) + asyncio.run(run()) + assert "host" not in seen From b5bfb944866dcae6b7120886a383fe3961d5edf6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=98=A4=EC=A4=80=EC=84=9D=28Junseok=20Oh=29DevOps?= <52226147+Atom-oh@users.noreply.github.com> Date: Mon, 19 Oct 2026 23:23:47 +0900 Subject: [PATCH 07/27] fix(web): show datasource completeness, limits and shared scope (#185) * fix(web): show datasource completeness, limits and shared scope The datasource API and UI report instance-specific configuration only, show effective execution/response limits, pass the selected window to trace and SaaS kinds, reject unsupported SQL range shapes, and label shared default datasources. Co-Authored-By: Claude Opus 5.5 * fix(web): align datasource guides with the stacked connector contract Review follow-ups: the datasource guides (ko/en/ja/zh) and API reference describe the shared timeout contract, the real-query connection test and the Mimir API prefix; the reference doc records Loki saturation disclosure, the shared request deadline and the oauth_3lo posture. The form no longer offers oauth_3lo. A Datadog instance's legacy key pair counts as configured and survives a settings-only PATCH regardless of the row's authType. Co-Authored-By: Claude Fable 5.1 * fix: address review feedback (iteration 1/5) * fix: address review feedback (iteration 2/5) * fix(web): defer oauth_3lo enablement to the custom MCP governance gate The enable-time transport check is superseded by the merged default-off governance gate, which admits only api_key and oauth_client_credentials; registration validation and the form still reject oauth_3lo. Co-Authored-By: Claude Fable 5.1 --------- Co-authored-by: Claude Opus 5.5 --- CHANGELOG.md | 8 +- docs/api-reference.md | 6 +- docs/reference/README.md | 6 + docs/reference/external-connections.md | 135 ++++++++++++++++++ web/app/api/datasources/authz.test.ts | 2 +- web/app/api/datasources/manage/route.test.ts | 22 +++ web/app/api/datasources/manage/route.ts | 19 ++- web/app/api/datasources/query/route.test.ts | 8 +- web/app/api/datasources/query/route.ts | 60 ++++---- web/app/api/datasources/query/window.test.ts | 70 +++++++++ web/app/api/datasources/route.test.ts | 19 ++- web/app/api/datasources/route.ts | 14 +- web/app/api/datasources/test/route.test.ts | 14 +- web/app/api/datasources/test/route.ts | 25 +++- web/app/customization/page.tsx | 4 +- .../datasources/DatasourceForm.tsx | 30 +++- .../datasources/DatasourcesTab.test.tsx | 14 +- .../datasources/DatasourcesTab.tsx | 13 +- web/components/datasources/ExplorePanel.tsx | 16 ++- web/lib/datasource-query-tools.ts | 15 +- web/lib/datasource-render.test.ts | 15 ++ web/lib/datasources.ts | 9 +- web/lib/i18n-terms.ts | 11 ++ web/lib/integration-credentials.id.test.ts | 56 ++++++++ web/lib/integration-credentials.ts | 37 +++++ web/lib/integration-validation.test.ts | 6 + web/lib/integration-validation.ts | 1 + 27 files changed, 550 insertions(+), 85 deletions(-) create mode 100644 docs/reference/external-connections.md create mode 100644 web/app/api/datasources/query/window.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index afce8dd77..094822f3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,7 +57,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Account-scoped resource trend + derived security-count history: the daily inventory snapshot is now written per account (over the sync's trusted-account set — an unreachable account keeps its last same-day row, a reachable-but-empty account records a genuine 0), the trend API accepts the accounts scope (validated CSV; `__all__` resolves server-side to self + the enabled member accounts — never an unfiltered read, which would sum backfill 'aggregate' rows and offboarded-account history) and returns per-day PER-TYPE account coverage, and the home trend chart, delta table, 7d net change, and cost-impact estimate all follow the account selector — the guards additionally require each compared type's coverage to equal the scope that type can ever reach (host-only SDK-collected types like S3 check against the host account; everything else against the full resolved scope — the sync runs per type, so an account can be silent for one type's run only, even on both compared days), so a silent (account, type) day — or the deploy boundary, before per-account history exists — renders '—' in the KPI and delta table, hides the cost-impact panel, and draws as a line gap in the chart (with a disclosure caption) instead of a fabricated fleet change; a CSV selection narrowed by dropped ids, or the all-accounts fallback when the accounts registry is unreadable, is disclosed under the chart, and the cost-impact panel hides whenever its own fetch degraded or resolved a different scope than the chart's (per-account history accrues from this deploy; snapshots still carry no region dimension, so a narrowed region scope keeps hiding the trend-derived KPIs). The sync also historizes three derived security series — Public S3 Buckets, Open Security Groups, Unencrypted EBS — by counting the just-synced inventory with the Security page's own predicates (lockstep-guarded), shown as delta rows and as the chart's own default-hidden Security-series toggle group (appended after the top real types so they are always reachable) but excluded from the day's total to avoid double-counting; EKS/K8s counts are deliberately not historized (v2 has no batch K8s collection — EKS reads are live on-request; ECS tasks/services already trend as their own synced types). - Inventory/datasources i18n completion: the generic inventory pages (CloudFront/DynamoDB/WAF and every other type) and the datasources hub (form, tab, card dashboard, Explore, log view) now translate their Korean UI strings into en/zh/ja — 17 unregistered literals, the dynamic card/note catalogs (card_catalog titles, datasource-render result notes), the log-view caption pattern, and the donut/chart title patterns were registered; donut titles are composed fully in Korean so one tt() pass translates them (pre-translating the sample suffix produced untranslatable mixed strings); the two English action buttons (Add datasource / Test connection) are localized. A ratchet test pins the static-literal coverage on those surfaces (dynamic strings are covered by the registered catalogs and RULES — the test is a regression guard, not a completeness proof). Column/spec labels deliberately stay English. - Full-fleet aggregates past the 500-row cap: capped inventory pages fetch server-side GROUP BYs (state tiles, distribution donuts, state-filter and facet dropdown options, and the exact total) over the WHOLE scoped fleet — v1 ran its aggregation SQL fleet-wide, and the v2 sample-based counts were silently inaccurate above 500 resources. Coverage is per-dimension: dimensions whose values are client-derived (lambda's runtime and dynamodb's billing donuts, ecs_task cluster/cpu/memory facets, opensearch's encryption-status donut, msk's kafka-version facet) stay sample-based, and an option list with 50+ distinct values falls back to the sample — every sample-based donut now discloses itself with the '(표본 기준)' qualifier (previously unlabeled). Full-fleet donut 'Other' buckets are computed against the fleet total, so they sum correctly regardless of the server's bucket cap. One aggregation call replaces the previous true-total summary call; the table, Top-N bars, and highlight cards deliberately stay sample-based. -- Per-datasource connection settings: the datasource form gains a Settings section — an upstream query timeout (seconds 1–60, default 10; forwarded as the Prometheus/Mimir API timeout param capped under the connector's HTTP timeout, and as ClickHouse max_execution_time) and a ClickHouse default database (identifier-only, validated on both the web tier and the connector before any HTTP call) — persisted per instance — the ClickHouse bound is the CEILING on every path (Explore, service graph, agent/worker — callers can only tighten it; default 10s, connector HTTP timeout aligned above it) while the Prometheus/Mimir bound applies on the Explore path capped at 10s, and the database rejects system/information_schema on both validation layers; v1's result-cache TTL is deliberately not ported (the v2 query path is uncached by design) and the timeout unit changed from ms to seconds, both disclosed in the guide. ClickHouse settings of 56–60s have an effective 55s ceiling under the Lambda wall limit; the guide distinguishes stored values from effective limits. +- Per-datasource connection settings: instance settings distinguish provider execution ceilings for Prometheus/Mimir/ClickHouse (default 10s, enforced inside the connector on every query path; callers can only tighten them) from HTTP response timeouts for other kinds (default 12s, without a server-cancellation guarantee). Stored 1–60s values have an effective 55s maximum and the form shows the applied limit. ClickHouse database identifiers reject system/information_schema on both validation layers; Mimir's API prefix is configurable without duplicating an existing mount. Missing explicit instance credentials fail closed, and upstream success/error bytes are bounded before parsing. Query-access tests run one real bounded query (readiness endpoints no longer pass) and report latency and the verified operation with fixed sentences; Jaeger/Dynatrace/Datadog distinguish malformed responses from empty results, and Jaeger and Loki report reaching the requested limit as potentially partial with requested/returned counts, separately from dropped records. A connector request shares one deadline across connect, headers and body. The default instance's shared credential is stamped with its instance id so a sibling instance can never borrow it; only the no-ID legacy default path remains compatible, and migrated defaults require admin backfill before credential-blind producers roll out. Optional Jaeger/Dynatrace private networking remains default-off. Custom MCP retains admin-managed extensibility with validated-address pinning, original Host/SNI/certificate verification and no redirects; unsupported three-legged OAuth is rejected. The v2 query path remains uncached and timeout units remain seconds rather than v1 milliseconds. - Unified ECS overview page (/inventory/ecs, 'ECS Overview' in the sidebar's ECS subgroup): v1's one-screen posture restored — a summary KPI band (cluster/service counts from the loaded pages ['+' at the 500 cap], task count from the shared summary, and a tasks-below-desired attention tile aggregated only over an untruncated service page — a sample never presents a fleet total), plus the clusters and services tables stacked on one screen; each table links to its full type page for search/facets/detail (the overview is a read-only glance layer), labels a >=500-row page as a sample, shows a stale-data caption under a failed sync, and reads 'not collected yet' pre-sync instead of a fabricated empty fleet. - Dashboard on-demand sync: the header gains an admin-only 'Sync all' action that dispatches one all-types inventory sync (async batch semantics disclosed in place — a queued acknowledgement [enqueue only, not a completion guarantee; already-running types are skipped], admin-only and sync-disabled states rendered distinctly from transient failures; data lands via the normal Refresh minutes later, no optimistic mutation). - S3 security drill-downs: the S3 page gains v1's Bucket Map by Region (block tiles colored Public=red > Versioned=green > Standard=cyan with an explicit Unknown=gray — a bucket whose policy flag is unknown stays Unknown even when versioning is known (a denied policy lookup must not paint a reassuring color), never a confident Standard; block click opens the detail panel; sample-labeled past the 500-row cap), and the bucket detail gains an 'IAM Roles with S3 Access' section (ADMIN-ONLY — non-admins see a permission note; roles whose newly synced attached AWS-managed policies match the checked set [AmazonS3*/AdministratorAccess/PowerUserAccess/ReadOnlyAccess incl. job-function paths — other policies can also grant S3, so the empty state is matched-set-framed], max 30; the last sync run's status gates every conclusion — a failed/partial run shows a stale-data banner and the empty state is only conclusive under a succeeded run within 24h on an untruncated page (a data-as-of timestamp rides the footer); a 500-row sample is labeled; inline/bucket-policy access is explicitly out of scope, and pre-sync rows show a 'not synced' note rather than an empty all-clear — visible after a terraform apply + the next sync; note: a failed policy-list hydrate — an SCP block, or a fleet whose aggregate role count exceeds the sync's rate-limit budget — retries without the optional attached-policy column (other hydrates remain and may still fail) a successful fallback refreshes base iam_role rows and only this section shows 'not synced' (the run reports degraded freshness so readers see the blind spot); the whole-run last-good freeze on this path requires the base query to also fail — the final run status otherwise follows the normal sync lifecycle — the ADR-010 amendment's disclosed semantics). @@ -95,7 +95,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - AI-diagnosis generation UX quick wins (v1 parity): a running diagnosis shows an mm:ss elapsed timer and a per-section checklist grid (completed / pending, in the UI language — driven by a new additive `completed` list the worker streams into the progress JSONB; no per-section spinner on purpose, since concurrent rendering leaves no in-flight telemetry to show; older in-flight rows and a drifted section catalog fall back to the bar-only view), a completed report shows a stats bar (section count · duration · report id — duration comes from a new `finished_at` column stamped at terminal write [one additive migration]; legacy rows without it omit the segment, never fabricated), the empty state previews the full section scope with Deep-tier tags, and completed history rows carry inline MD/DOCX download links (no need to open the report). - AI-diagnosis parity batch (v1 parity): a completed report now renders as collapsible section cards with a sticky table-of-contents sidebar (click scrolls to the section) and a per-section severity icon derived from body keywords (a display heuristic, labeled as such — not a score; reports without section headings keep the continuous view); report generation language is selectable (한국어/English/中文/日本語 — defaults to the UI language, applies to manual runs and schedules; the language is part of the run's dedup key, so a same-hour language switch starts a new run instead of returning the previous language's report; a legacy dedup-key read fallback ships for one release for rolling-deploy compatibility — REMOVE in the release after this one); the auto-diagnosis schedule gains KST detail settings (weekday for weekly/biweekly, day-of-month 1–28 for monthly, run hour) plus a last-run timestamp display — unset fields keep the previous interval-only behavior; and admins can send a test notification to the diagnosis mailing list from the subscribers panel (one SNS publish scoped to the existing diagnosis topic — the web task's first, admin-only `sns:Publish`; a failed send surfaces its error, never a silent success). - Inventory KPI/chart quick-win batch (v1 parity): EC2 gains a running total-vCPU tile (per-instance `cpu_options` cores×threads — actual vCPUs, not the type default); RDS a total allocated-storage tile; Lambda long-timeout (>300s, danger) and average-memory tiles; EBS volumes an encryption-rate tile (100% → accent, <80% → danger); ECS clusters get a dedicated KPI band (ACTIVE count, running tasks, active services, container instances) instead of the generic state tiles; ECR rows gain a Scan on Push column (a missing/malformed scanning config counts as No — the API default); and the CloudWatch alarm-state donut uses fixed semantic colors (green OK / red ALARM / gray INSUFFICIENT_DATA) instead of size-ordered palette colors. -- Datasource Explore/management parity batch: curated example-query and natural-language prompt chips for all 8 connector kinds, a dedicated Loki log-stream viewer (timestamps, label badges, scrollable pane), 7d/30d time-range presets (per-kind API bound: prometheus/mimir 30d with an upstream `timeout` forwarded by their connectors — the connector change ships via `terraform apply`, so run the apply before relying on 30d; Loki capped at 7d; the 5000-point density cap stays), a result metadata bar (rows/series · execution ms · query language · shape), a dismissible "AI generated from …" banner after NL→query drafting, KPI tiles and a manual refresh button on the management tab, an "AI로 진단" deep link on each kind's DEFAULT datasource row (the chat tool path resolves per-kind defaults; supported kinds only) that prefills the assistant composer with a section-pinned prompt (`/assistant?q=`, review-only — never auto-sends), and proportional duration bars on Tempo/Jaeger trace results. +- Datasource Explore/management parity batch: curated example-query and natural-language prompt chips for all 8 connector kinds, a dedicated Loki log-stream viewer (timestamps, label badges, scrollable pane), 7d/30d time-range presets (Prometheus/Mimir/Dynatrace/Datadog up to 30d; Loki/Tempo/Jaeger up to 7d; the 5000-point density cap stays), a result metadata bar (rows/series · execution ms · query language · shape), a dismissible "AI generated from …" banner after NL→query drafting, KPI tiles and a manual refresh button on the management tab, an "AI로 진단" deep link on each kind's DEFAULT datasource row (the chat tool path resolves per-kind defaults; supported kinds only) that prefills the assistant composer with a section-pinned prompt (`/assistant?q=`, review-only — never auto-sends), and proportional duration bars on Tempo/Jaeger trace results. Search windows are forwarded independently of range-tool availability, SQL API ranges are rejected rather than ignored, and effective request bounds/shared datasource scope are explicit. AI drafting does not change the selected window. Datasource status reports verified configuration separately from untested connectivity, without inferring connection from default selection. Datadog connection checks exercise metric-query access for key pairs and bearer tokens; Datadog point caps and Dynatrace nested series caps or advertised compatibility pages remain visibly partial in Explore. Deploy the Lambda producers before the dependent Web changes. - Datasource detail pages gain a pre-built card dashboard: registering a datasource (and each daily index run) derives an expected card set from the cached schema — the queries each card uses are stored ahead of time (new `datasource_dashboard_cards` table + a deterministic `card_catalog` in the index worker; prometheus/mimir 13 cards covering targets, CPU, memory, disk, load, network, containers, and restarts; loki 2, tempo 2, clickhouse 2) — ready Prometheus/Mimir cards are live-validated against the exact datasource before registration, through the same instant/range tool the page will execute (a conclusive PromQL error — body-derived, never a bare HTTP 4xx — disables only that card and is revalidated on the next daily run; a transient connector failure, a failed re-introspection, or a truncated schema that cannot decide a card requirement all preserve the previous card set; the connector metric-metadata tool also gains a definitive `exists` flag and 3-second upstream deadlines) — card building runs inside the existing datasource-index job, so it is gated on `datasource_diagnosis_enabled` (default false; requires `workers_enabled`/`agentcore_enabled`/`integrations_enabled`) like the diag-signal chips — and the page executes the stored queries live on view through the existing read-only query API (stat/timeseries/table cards; unavailable cards render dimmed with what's missing; a failed card shows an inline error, never a silent zero). - Topology infra page gains two columnar map views — a 5-column infra resource map (External | VPC | Subnet | Compute | NAT) and a per-cluster K8s map (Ingress → Service → Pod → Node; host-account, connected clusters only — the in-cluster read path is host-scoped) — rendered as a real graph (fixed-column ReactFlow with edge lines), with click cross-highlighting, search highlighting, and a color legend. Built on existing inventory/EKS reads plus the pre-existing read-only `/api/tgw` live attachment describe (host-account scope only, first 20 TGWs — degradation is surfaced in the UI); the only server-side addition is the read-only `ingresses` in-cluster kind. @@ -726,7 +726,7 @@ First release of the **v2 line** (versioned independently from the v1 1.x line, - 계정별 리소스 추이 + 파생 보안 카운트 이력화: 일별 인벤토리 스냅샷을 계정별로 기록(sync의 신뢰 계정 집합 기준 — 미도달 계정은 당일 기존 행 보존, 도달했지만 0건인 계정은 진짜 0 기록), 추이 API가 accounts 스코프를 수용(검증된 CSV; `__all__`은 서버에서 self+활성 멤버 계정으로 해석 — 필터를 걷어내지 않음: 무필터 조회는 백필의 'aggregate' 행과 오프보딩된 계정 이력까지 합산)하고 일자·타입별 계정 커버리지를 반환, 홈 추이 차트·수량 변화 테이블·7일 순증감·비용 영향 추정이 계정 선택을 따름 — 가드는 비교 두 시점에서 각 타입의 커버리지가 그 타입이 도달 가능한 스코프와 정확히 일치할 것을 추가로 요구(S3 등 호스트 전용 SDK 수집 타입은 호스트 계정 기준, 나머지는 해석된 전체 스코프 기준 — sync는 타입별로 돌므로 한 계정이 특정 타입 run에서만, 두 시점 모두에서도, 침묵 가능)해, 침묵한 (계정, 타입) 일자나 계정별 이력이 없는 배포 경계는 플릿 변화를 지어내지 않고 KPI·변화 테이블은 '—', 비용 영향 패널은 숨김, 차트는 해당 시점을 공백(라인 갭)으로 표시(공지 캡션 포함); 유효하지 않은 ID로 좁혀진 CSV 선택이나 계정 레지스트리 조회 실패 시의 전체 계정 폴백은 차트 아래에 공지되고, 비용 영향 패널은 자체 조회가 폴백됐거나 차트와 다른 스코프로 해석된 경우 숨김(계정별 이력은 본 배포 이후부터 축적; 스냅샷에는 여전히 리전 차원이 없어 리전 스코프를 좁히면 추이 기반 KPI는 계속 숨김). sync가 파생 보안 시리즈 3종(Public S3 Buckets·Open Security Groups·Unencrypted EBS)을 보안 페이지의 판정 술어 그대로(락스텝 가드) 방금 동기화된 인벤토리에서 COUNT해 이력화 — 변화 테이블 및 차트의 기본 숨김 '보안 시리즈' 토글 그룹(상위 실제 타입 뒤에 항상 추가되어 접근 가능)으로 표시되지만 이중 계산 방지를 위해 일별 total에서는 제외. EKS/K8s 카운트는 의도적으로 미이력화(v2에는 K8s 배치 수집이 없음 — EKS는 온디맨드 라이브 조회; ECS tasks/services는 자체 동기화 타입으로 이미 추이 존재). - 인벤토리/데이터소스 i18n 완결: 범용 인벤토리 페이지(CloudFront/DynamoDB/WAF 포함 전 타입)와 데이터소스 허브(폼·탭·카드 대시보드·Explore·로그 뷰)의 한국어 UI 문자열이 en/zh/ja로 번역됨 — 미등록 리터럴 17건, 동적 카드/노트 카탈로그(card_catalog 제목, datasource-render 결과 note), 로그 뷰 캡션 패턴, 도넛/차트 제목 패턴을 등록. 도넛 제목은 완전 한국어로 조합해 tt() 1회로 번역(표본 접미사 선번역은 번역 불가한 혼합 문자열을 만들었음), 영문 액션 버튼 2종(Add datasource/Test connection) 현지화. 래칫 테스트가 정적 리터럴 커버리지를 고정(동적 문자열은 등록된 카탈로그와 RULES로 커버 — 완전성 증명이 아닌 회귀 방지 장치). 컬럼/스펙 라벨은 의도적으로 영어 유지. - 500행 캡 초과 시 전수 집계: 캡에 도달한 인벤토리 페이지가 서버 측 GROUP BY(상태 타일, 분포 도넛, 상태 필터·패싯 드롭다운 옵션, 정확한 총계)를 전체 스코프 플릿 기준으로 조회 — v1은 집계 SQL을 전 플릿에 실행했고 v2의 표본 기반 수치는 500대 초과 시 조용히 부정확했음. 커버리지는 차원별: 클라이언트 파생 값 차원(lambda runtime·dynamodb billing 도넛, ecs_task cluster/cpu/memory 패싯, opensearch 암호화 상태 도넛, msk kafka 버전 패싯)은 표본 유지, 고유값 50개 이상 옵션 목록도 표본 폴백 — 표본 기반 도넛은 모두 '(표본 기준)'으로 자체 공지(종전엔 무표기). 전수 도넛의 '기타'는 플릿 총계 기준으로 계산되어 서버 버킷 캡과 무관하게 합계가 맞음. 집계 호출 1회가 기존 총계용 summary 호출을 대체하며, 테이블·Top-N 바·하이라이트 카드는 의도적으로 표본 유지. -- 데이터소스별 연결 설정: 데이터소스 폼에 Settings 섹션 추가 — 업스트림 쿼리 타임아웃(초 1–60, 기본 10; Prometheus/Mimir는 API timeout 파라미터로 커넥터 HTTP 타임아웃 아래로 캡, ClickHouse는 max_execution_time)과 ClickHouse 기본 database(식별자만, 웹 계층과 커넥터 양쪽에서 HTTP 호출 전 검증) — 인스턴스별로 저장 — ClickHouse 제한은 모든 경로(Explore·서비스 그래프·에이전트/워커)의 상한으로 적용되고(호출자는 더 짧게만 조정 가능; 기본 10초, 커넥터 HTTP 타임아웃을 그 위로 정렬) Prometheus/Mimir 제한은 Explore 경로에 10초 캡으로 적용되며, database는 system/information_schema를 양쪽 검증 계층에서 거부. v1의 결과 캐시 TTL은 의도적으로 미이식(v2 질의 경로는 무캐시 설계), 타임아웃 단위는 ms→초 변경 — 모두 가이드에 공지. ClickHouse의 56–60초 설정은 Lambda 한도 때문에 유효 55초로 단축되며 가이드에서 저장 값과 실제 상한을 구분한다. +- 데이터소스별 연결 설정: 인스턴스 설정에서 Prometheus/Mimir/ClickHouse의 서버 실행 제한(기본 10초, 모든 쿼리 경로의 커넥터에서 강제하며 호출자는 더 짧게만 조정 가능)과 나머지 종류의 HTTP 응답 제한(기본 12초, 서버 쿼리 중단 보장 없음)을 구분합니다. 저장 값 1–60초의 실제 상한은 55초이며 폼에서 적용 제한을 표시합니다. ClickHouse database는 양쪽 검증 계층에서 system/information_schema를 거부하고, Mimir API 접두사는 기존 경로와 중복되지 않게 설정할 수 있습니다. 명시한 인스턴스 자격증명이 없으면 실패하며 상위 성공·오류 응답은 파싱 전에 바이트 제한을 적용합니다. 연결 테스트는 실제 bounded 쿼리 1회를 실행해(readiness 엔드포인트만으로는 통과하지 않음) 지연 시간과 검증한 작업을 고정 문구로 보고하며, Jaeger/Dynatrace/Datadog는 잘못된 응답과 빈 결과를 구분하고, Jaeger와 Loki는 요청 상한 도달을 요청/반환 개수와 함께 ‘부분 가능’으로 표시해 실제 누락과 구분합니다. 커넥터 요청은 연결·헤더·본문이 하나의 제한 시간을 공유합니다. 기본 인스턴스의 공유 자격증명에는 인스턴스 id를 기록해 다른 인스턴스가 빌려 쓸 수 없으며, ID 없는 레거시 기본 경로만 호환되며, 마이그레이션된 기본 인스턴스는 자격증명 정보를 받지 않는 호출 측을 배포하기 전에 관리자가 백필해야 합니다. Jaeger/Dynatrace 내부망 옵션은 기본 OFF입니다. 커스텀 MCP는 관리자 확장성을 유지하면서 검증한 주소로 접속하고 원래 Host/SNI·인증서 검증과 리다이렉트 차단을 유지하며, 지원하지 않는 3-legged OAuth는 거부합니다. v2 쿼리 경로는 계속 무캐시이고 Timeout 단위는 v1의 밀리초가 아닌 초입니다. - ECS 통합 개요 페이지(/inventory/ecs, 사이드바 ECS 서브그룹의 'ECS 개요'): v1의 한 화면 뷰 복원 — 요약 KPI 밴드(클러스터/서비스 수는 로드된 페이지 기준[500 캡 도달 시 '+'], 태스크 수는 공용 summary, Desired 대비 미달 태스크 타일은 비절단 서비스 페이지에서만 집계 — 표본을 전체 합계처럼 제시하지 않음)와 클러스터·서비스 테이블을 한 화면에 세로로 배치. 각 테이블은 검색/패싯/상세가 있는 타입 페이지로 '전체 보기' 링크(개요는 읽기 전용 글랜스 레이어), 500행 이상은 표본 표기, sync 실패 시 오래된 데이터 캡션, 미수집 시 '미수집' 표시(빈 플릿 조작 없음). - 대시보드 온디맨드 동기화: 헤더에 관리자 전용 '전체 동기화' 버튼 추가 — 전체 타입 인벤토리 sync를 1회 dispatch(비동기 배치 시맨틱을 그 자리에서 공지 — 큐 등록 확인일 뿐 완료 보장이 아니며 이미 실행 중인 타입은 건너뜀; 관리자 전용·sync 비활성 상태를 일시 오류와 구분해 표시; 데이터는 수 분 후 일반 Refresh로 반영, 낙관적 갱신 없음). - S3 보안 드릴다운: S3 페이지에 v1의 리전별 버킷 맵(블록 타일 — Public=빨강 > Versioned=초록 > Standard=시안, 플래그 미동기화 버킷은 명시적 Unknown=회색으로 표시해 확정 Standard로 읽히지 않음; 블록 클릭 시 상세 패널, 500행 캡 초과 시 표본 기준 표기; Public 플래그 미상 버킷은 버저닝이 알려져 있어도 Unknown — 거부된 정책 조회가 안심 색으로 가려지지 않음), 버킷 상세에 'S3 접근 권한 보유 IAM Role' 섹션(관리자 전용 — 비관리자에겐 권한 안내 표시; 신규 sync되는 연결 AWS 관리형 정책이 검사 세트[AmazonS3*/AdministratorAccess/PowerUserAccess/ReadOnlyAccess, job-function 경로 포함]에 일치하는 경우 최대 30개 — 다른 정책도 S3를 부여할 수 있어 빈 결과는 검사-세트 한정 문구; 마지막 sync run 상태가 모든 결론을 게이트 — 실패/부분 run에는 오래된 데이터 배너, 빈 결과는 24시간 내 성공·비절단 run에서만 확정(하단에 기준 시각 표기); 500행 표본 표기; 인라인/버킷 정책 경유 접근은 범위 밖임을 명시, 정책 목록 sync 전에는 '미동기화' 안내 — terraform apply + 다음 sync 후 표시; 참고: 정책 목록 하이드레이트 실패(SCP 차단, 또는 전 계정 합산 role 수가 rate-limit 예산 초과) 시 선택적인 연결 정책 열을 제외해 재시도하며(다른 추가 조회는 남아 있어 실패할 수 있음) 기본 iam_role 인벤토리는 유지되고 이 섹션만 '미동기화'로 표시(run은 degraded freshness로 공개) — 이 경로의 run 전체 failed·전 계정 last-good 동결은 기본 쿼리까지 실패한 경우이며 그 외 최종 run 상태는 통상 sync 라이프사이클을 따름 — ADR-010 개정의 공지된 시맨틱) 추가. @@ -763,7 +763,7 @@ First release of the **v2 line** (versioned independently from the v1 1.x line, - AI 진단 생성 UX 퀵윈(v1 패리티): 생성 중 mm:ss 경과 타이머와 섹션별 체크리스트 그리드(완료/대기 2단계, UI 언어로 표시 — 워커가 progress JSONB에 추가로 스트리밍하는 `completed` 목록 기반; 동시 렌더 특성상 진행 중 섹션 텔레메트리가 존재하지 않아 스피너는 의도적으로 제공하지 않음; 이전 형식의 진행 중 행·카탈로그 드리프트 시 기존 진행 바 뷰로 폴백), 완료 리포트에 통계 바(섹션 수 · 소요 · 리포트 ID — 소요는 종료 시점에 기록되는 신규 `finished_at` 컬럼[추가 마이그레이션 1건] 기반; 값이 없는 레거시 행은 소요를 생략하고 절대 임의 산출하지 않음), 빈 상태에 Deep 티어 태그가 붙은 전체 섹션 범위 프리뷰, 완료된 히스토리 행에 인라인 MD/DOCX 다운로드 링크(리포트를 열지 않고 즉시 다운로드) 추가. - AI 진단 패리티 배치(v1 패리티): 완료 리포트를 접을 수 있는 섹션 카드 + 고정 목차 사이드바(클릭 시 해당 섹션으로 스크롤) + 본문 키워드 기반 섹션별 심각도 아이콘(점수가 아닌 표시 휴리스틱임을 명시; 섹션 헤딩이 없는 리포트는 기존 연속 뷰 유지)으로 렌더링; 리포트 생성 언어 선택(한국어/English/中文/日本語 — UI 언어 기본값, 수동 실행·스케줄 모두 적용; 언어가 실행 dedup 키에 포함되어 같은 시간대 언어 전환 시 이전 언어 리포트를 재사용하지 않고 새로 실행; 롤링 배포 호환을 위한 레거시 dedup 키 read 폴백이 이번 릴리스 한정으로 포함 — 다음 릴리스에서 제거); 자동 진단 스케줄에 KST 상세 설정(매주/격주 요일, 매월 1–28일, 실행 시각)과 최근 실행 시각 표시 추가 — 미설정 필드는 기존 주기-간격 동작 유지; 구독자 패널에서 관리자가 테스트 알림을 발송 가능(기존 진단 토픽 한정 SNS publish 1건 — web 태스크 최초의 관리자 전용 `sns:Publish`; 발송 실패는 조용한 성공이 아니라 에러로 표시). - 인벤토리 KPI/차트 퀵윈 배치(v1 패리티): EC2에 실행 중 총 vCPU 타일(인스턴스별 `cpu_options` 코어×스레드 — 타입 기본값이 아닌 실제 vCPU), RDS에 총 할당 스토리지 타일, Lambda에 장기 타임아웃(>300s, danger)·평균 메모리 타일, EBS 볼륨에 암호화율 타일(100% → accent, <80% → danger) 추가; ECS 클러스터는 일반 상태 타일 대신 전용 KPI 밴드(ACTIVE 수·실행 태스크·활성 서비스·컨테이너 인스턴스)를 표시; ECR 행에 Scan on Push 컬럼 추가(스캔 설정 누락/파싱 불가 시 API 기본값인 No로 집계); CloudWatch 알람 상태 도넛은 크기순 팔레트 색 대신 고정 시맨틱 컬러(초록 OK / 빨강 ALARM / 회색 INSUFFICIENT_DATA)를 사용. -- 데이터소스 Explore/관리 패리티 배치: 8개 커넥터 타입 전부에 큐레이트 예제 쿼리·자연어 프롬프트 칩, Loki 전용 로그 스트림 뷰어(타임스탬프·라벨 배지·스크롤 패널), 7d/30d 기간 프리셋(kind별 API 상한: prometheus/mimir는 커넥터가 업스트림 `timeout`을 전달하는 조건으로 30d — 커넥터 변경은 `terraform apply`로 배포되므로 30d 의존 전 apply 필요; Loki는 7d; 5000-포인트 밀도 캡 유지), 결과 메타데이터 바(행/시리즈 수 · 실행 ms · 쿼리 언어 · 형태), NL→쿼리 생성 후 닫을 수 있는 "AI 생성됨" 배너, 관리 탭 KPI 타일과 수동 새로고침 버튼, kind별 DEFAULT 데이터소스 행의 "AI로 진단" 딥링크(챗 도구 경로가 kind별 default를 해석하므로 default 행·지원 kind 한정, 섹션 고정 프롬프트로 `/assistant?q=` 컴포저 프리필 — 검토 전용, 자동 전송 없음), Tempo/Jaeger 트레이스 결과의 비례 duration 바 추가. +- 데이터소스 Explore/관리 패리티 배치: 8개 커넥터 타입 전부에 큐레이트 예제 쿼리·자연어 프롬프트 칩, Loki 전용 로그 스트림 뷰어(타임스탬프·라벨 배지·스크롤 패널), 7d/30d 기간 프리셋(Prometheus/Mimir/Dynatrace/Datadog 최대 30일, Loki/Tempo/Jaeger 최대 7일, 5000-포인트 밀도 상한 유지), 결과 메타데이터 바(행/시리즈 수 · 실행 ms · 쿼리 언어 · 형태), NL→쿼리 생성 후 닫을 수 있는 "AI 생성됨" 배너, 관리 탭 KPI 타일과 수동 새로고침 버튼, kind별 DEFAULT 데이터소스 행의 "AI로 진단" 딥링크(지원 kind 한정, `/assistant?q=` 컴포저 프리필, 검토 전용·자동 전송 없음), Tempo/Jaeger 트레이스 결과의 비례 duration 바를 제공합니다. 검색 기간은 별도 range 도구 유무와 관계없이 전달하고 SQL API 기간은 무시하지 않고 거부하며 실제 요청 기간과 공유 데이터소스 범위를 명시합니다. AI 생성은 선택한 기간을 변경하지 않습니다. 데이터소스 상태는 기본 선택 여부로 연결을 추정하지 않고 확인된 설정과 미확인 연결을 구분합니다. Datadog 연결 테스트는 키 쌍과 bearer 토큰의 메트릭 조회 접근까지 확인하며 Datadog 포인트 상한과 Dynatrace 내부 시리즈 상한·호환 응답의 추가 페이지는 부분 결과로 표시합니다. 의존하는 Web 변경보다 Lambda producer를 먼저 배포해야 합니다. - 데이터소스 상세 페이지에 사전 생성 카드 대시보드 추가: 등록 시(및 일일 인덱스 배치마다) 캐시된 스키마로부터 예상 카드 세트를 도출하고 각 카드가 사용할 쿼리를 미리 저장(신규 `datasource_dashboard_cards` 테이블 + 인덱스 워커의 결정론적 `card_catalog` — prometheus/mimir는 타깃·CPU·메모리·디스크·로드·네트워크·컨테이너·재시작을 포괄하는 13종, loki 2종·tempo 2종·clickhouse 2종) — ready Prometheus/Mimir 카드는 등록 전에 해당 데이터소스에서, 페이지가 실제 실행할 instant/range 툴 그대로 라이브 검증하며(확정 PromQL 오류 — 응답 본문 기반, 단순 HTTP 4xx 아님 — 는 해당 카드만 비활성화하고 다음 일일 실행에서 재검증, 일시적 커넥터 장애·재수집 실패·카드 요구 메트릭을 판정할 수 없는 절단 스키마는 모두 기존 카드 세트 보존; 커넥터 metric-metadata 툴에는 확정 `exists` 플래그와 업스트림 3초 제한 추가) — 카드 빌드는 기존 datasource-index 잡 내부에서 실행되므로 diag-signal 칩과 동일하게 `datasource_diagnosis_enabled` 게이트(기본 false; `workers_enabled`/`agentcore_enabled`/`integrations_enabled` 선행) 하에 동작 — 페이지가 저장된 쿼리를 기존 read-only 쿼리 API로 조회 시점에 라이브 실행해 렌더링(stat/시계열/테이블 카드, 미충족 카드는 누락 항목과 함께 비활성 표시, 실패 카드는 조용한 0이 아니라 인라인 에러로 표시). - 토폴로지 인프라 페이지에 컬럼형 맵 뷰 2종 추가 — 5컬럼 인프라 리소스 맵(External | VPC | Subnet | Compute | NAT)과 클러스터별 K8s 맵(Ingress → Service → Pod → Node — in-cluster 조회 경로가 host 스코프라 host 계정의 connected 클러스터만 대상) — 고정 컬럼 ReactFlow에 실제 엣지 연결선을 그리는 그래프로 렌더링되며, 클릭 교차 하이라이트·검색 하이라이트·색상 범례 포함. 기존 인벤토리/EKS 조회와 기존 read-only `/api/tgw` 라이브 어태치먼트 조회(호스트 계정 스코프 한정·최대 20개 — 미조회 시 UI에 표시)를 사용하며, 서버 측 추가는 read-only `ingresses` in-cluster kind 1종뿐. diff --git a/docs/api-reference.md b/docs/api-reference.md index 3e9bcc036..0cf3b1264 100644 --- a/docs/api-reference.md +++ b/docs/api-reference.md @@ -379,11 +379,11 @@ reachability. | `/api/cost/detail` | GET | 서비스별 비용 상세 (`?service=` 필수, ≤100자) | verifyUser | | `/api/finops/findings` | GET | ADR-020 FinOps 기본 권장 엔진 — 미해결 findings + 최근 배치 실행(`finops_runs`) 조회, Aurora만 읽음(라이브 AWS 호출 없음). `finops_baseline_enabled=false`면 `{enabled:false, findings:[], lastRun:null}` | verifyUser | | `/api/customization` | GET, POST, PUT | Admin skill/agent catalog CRUD; invalid tool policy → 400; unavailable validation/read → 503 (see below) | verifyUser | -| `/api/datasources` | GET | 데이터소스 인스턴스 목록 — 크리덴셜 미노출 | verifyUser | +| `/api/datasources` | GET | 데이터소스 인스턴스 목록 — 크리덴셜 미노출. 행마다 `configured`(인스턴스 자체의 설정 근거만, 연결 성공 주장 아님)·`connectionStatus`·`scope: 'shared'`(기본 인스턴스는 타입별 공유 연결) 포함 | verifyUser | | `/api/datasources/generate` | POST | 자연어 → 쿼리 초안 생성 (리뷰용 — 절대 실행 안 함) | verifyUser | -| `/api/datasources/manage` | POST, PATCH | 인스턴스 생성/수정 + 크리덴셜 저장 (admin); `settings`(timeoutS 1–60[clickhouse 유효 최대 55]·clickhouse database)는 서버 측 sanitize 후 ds_settings JSONB에 저장 | verifyUser | +| `/api/datasources/manage` | POST, PATCH | 인스턴스 생성/수정 + 크리덴셜 저장 (admin); `settings`(timeoutS 1–60[모든 kind 유효 최대 55 — Prometheus/Mimir/ClickHouse는 서버 실행 제한, 그 외는 HTTP 응답 제한]·clickhouse `database`·mimir `apiPrefix`)는 서버 측 sanitize 후 ds_settings JSONB에 저장 | verifyUser | | `/api/datasources/query` | POST | 인스턴스 대상 read-only 쿼리 실행 (admin 아님 — 탐색용) | verifyUser | -| `/api/datasources/test` | POST | 저장 전 연결 probe — SSRF 가드 (admin) | verifyUser | +| `/api/datasources/test` | POST | 저장 전 연결 테스트 — 실제 bounded 쿼리 1회 실행(readiness 엔드포인트 아님), `{ok, latencyMs, verified, error?}` 반환, SSRF 가드 (admin) | verifyUser | | `/api/datasources/[id]` | DELETE | 인스턴스 삭제 — 스키마 캐시/크리덴셜 cascade, 기본값 재선정 (admin) | verifyUser | | `/api/datasources/[id]/cards` | GET | 사전 생성 대시보드 카드 조회 (read-only, auth) | verifyUser | | `/api/datasources/[id]/default` | POST | kind별 기본 인스턴스 지정 — 트랜잭션으로 기존 기본 해제 (admin) | verifyUser | diff --git a/docs/reference/README.md b/docs/reference/README.md index 1bb48f358..af0649c22 100644 --- a/docs/reference/README.md +++ b/docs/reference/README.md @@ -83,6 +83,12 @@ scoped registered-fleet coverage from incomplete wildcard discovery. **E2E observability — [observability-e2e.md](observability-e2e.md).** The opt-in `/topology?view=e2e` page connects account/region/global-scoped configuration, host service snapshots and explicit NFM queries using source-quality and scoped-identity gates. The reference distinguishes implemented contracts from the broader observability roadmap. +**External connections — [external-connections.md](external-connections.md).** The eight datasource +HTTP adapters and the administrator-managed custom MCP path: shared datasource scope, +explicit instance identity, bounded responses, query-level connection tests, completion +states and pinned custom-MCP transport. Registration, verification and deployment are +distinct states. + **VPC connectivity — [vpc-connectivity.md](vpc-connectivity.md).** On-demand peering and TGW attachment observations on `/inventory/vpc` and the `/topology/infra?view=vpc` tab, scoped to an enabled collecting account, region and diff --git a/docs/reference/external-connections.md b/docs/reference/external-connections.md new file mode 100644 index 000000000..ad6a9280d --- /dev/null +++ b/docs/reference/external-connections.md @@ -0,0 +1,135 @@ +# External observability connection contract + +This is the source-supported contract for the eight datasource HTTP adapters and +the separate administrator-managed MCP path. Registration, credential presence, +query verification, MCP provisioning and live deployment are different states. + +## Datasource scope and identity + +Datasource Explore is available to authenticated users. Create, edit, delete, +set-default, connection test and credential operations require administrator +authority. Built-in chat resolves one shared default per kind; selecting an AWS +account does not select a different datasource or tenant. + +Explore resolves credentials by instance ID and retains the row's endpoint, +authentication method and settings as authoritative. Lambda/worker requests with +an explicit instance ID fail when that instance is not configured; they cannot +borrow another instance's kind-default credential. The no-ID legacy default path +remains available. Secrets Manager's bounded credential cache does not provide +immediate revocation of previously cached credentials. + +Per-account custom integration selection and exposed-tool ceilings are enforced +separately. They do not restrict built-in shared datasource defaults. External +telemetry needs independent account/region evidence before identity correlation. + +## Capabilities and query windows + +| Kind | Query | Time window | Built-in chat | Private-network option | +|---|---|---|---|---| +| Prometheus | PromQL instant/range | Instant or up to 30 days | `external-obs` | `prometheus_vpc_enabled` | +| Mimir | PromQL instant/range | Instant or up to 30 days | `monitoring` | `mimir_vpc_enabled` | +| Loki | LogQL instant/range | Instant or up to seven days | `monitoring` | `loki_vpc_enabled` | +| Tempo | TraceQL search | One-hour default; up to seven days | `monitoring` | `tempo_vpc_enabled` | +| ClickHouse | Read-only SQL | Time predicates belong to SQL | `external-obs` Lambda | `clickhouse_vpc_enabled` | +| Jaeger | Service-anchored trace search | One-hour default; up to seven days | No built-in target | `jaeger_vpc_enabled` | +| Dynatrace | Environment Metrics API v2 metricSelector | One-hour default; up to 30 days | Separate official MCP preset | `dynatrace_vpc_enabled` | +| Datadog | Metrics REST API query | One-hour default; up to 30 days | Separate official MCP preset | Public endpoint | + +`POST /api/datasources/query` accepts `range: false`, legacy `range: true`, +or `{window, step}` in seconds. Search tools receive `start`/`end` independently +of whether a distinct range tool exists. Unsupported SQL API ranges are rejected. +Metadata includes the effective query window and shared scope. Range density +remains bounded; selecting a window does not guarantee complete provider coverage. +Natural-language drafting does not itself change the selected request window. + +Explore support does not establish service-graph support. Metrics-based graph +sources require suitable Prometheus/Mimir schema; trace sources require suitable +Tempo or ClickHouse data. Loki, Jaeger, Dynatrace and Datadog have no bundled +service-graph relationship adapter in the current catalog. + +## Endpoints and credentials + +Endpoints are HTTP(S) datasource bases, optionally including a reverse-proxy +mount. They are validated at registration and DNS-checked/pinned at connection. +Redirects and metadata/loopback/link-local targets are blocked. Datasource HTTP +permits private addresses; reachability still requires operator-managed networks. + +Mimir's `settings.apiPrefix` defaults to `/prometheus`. An empty value selects +`/api/v1` at the endpoint base. A non-empty prefix uses slash-separated letters, +digits, underscores and hyphens, up to 128 characters. If the endpoint already +ends in that prefix it is not appended twice. + +Generic datasource credentials support none/basic/bearer/custom headers and +an optional tenant header. Datadog's datasource API uses its API/application-key +pair; this differs from the official hosted MCP bearer PAT/SAT. Dynatrace's direct +environment adapter uses `Api-Token`; its official hosted MCP uses a bearer +Platform token. Official MCP credentials remain namespaced under `mcp:`. + +Jaeger currently uses its internal `/api/*` JSON compatibility interface, rather +than the stable v3 interface. A provider version change requires query-envelope +and time-unit validation before claiming compatibility. + +## Deadlines and completion + +Stored `timeoutS` remains an integer from 1 to 60 for compatibility. The effective +maximum is 55 seconds within the existing Lambda wall limit. + +- Prometheus/Mimir/ClickHouse apply a provider execution ceiling on query paths. + Default is 10 seconds; callers can tighten the ceiling. Their query HTTP + deadline is aligned above the execution bound. +- Loki/Tempo/Jaeger/Dynatrace/Datadog apply the saved `timeoutS` as an HTTP response + timeout, defaulting to 12 seconds. This does not establish provider-side query + cancellation. Connect, headers and body share that one deadline. +- Schema discovery may use shorter bounded operation deadlines. +- Success bodies are limited to 4 MiB before parsing; error bodies to 64 KiB. + Overflow fails explicitly and is never represented as an empty dataset. + +Connection tests use bounded reads against the actual supported query operation, +with the configured auth and tenant, and report `{ok, verified, latencyMs, error?}` +with fixed sentences (an SSRF policy rejection keeps its own classification). +Verified query access does not establish access to every metric, trace, dataset or +optional tool. + +Query producers distinguish verified complete `ok`/`empty` from `partial`, +`unknown` and `error`. Malformed/unrecognized responses cannot establish absence; +usable valid siblings may be retained with uncertainty. Jaeger and Loki disclose +the requested limit and returned count; reaching the requested limit is +`limitSaturated` and potentially partial even when no additional record is proven, +while `truncated` is reserved for records the connector actually dropped. +Actual HTTP 206 evidence is retained independently of upstream JSON fields and +cannot certify an empty dataset. + +## Private connectivity and MCP governance + +Every VPC option defaults false. Jaeger/Dynatrace's new options require both +AgentCore and integrations. An operator must separately verify private DNS, +subnet routing, target ingress and required AWS API access. No SG ingress or +Kubernetes grant is created by registering a datasource. + +Official MCP gates, exact read-tool allowlists and endpoint acknowledgements +remain governed by ADR-017. Dynatrace's empty official allowlist still authorizes +zero tools. ClickHouse's stdio official MCP remains frozen dark code. + +Custom external MCP registration remains allowed. Its runtime pins a request-local +validated address set with original Host/SNI and certificate verification, +rejects changed origins and redirects, disables proxy routing, and bounds +address fallback without replaying requests after read/write failures. +HTTPX/HTTPcore versions implementing the tested SNI contract are pinned in the +runtime dependencies. + +`oauth_3lo` is not executable: validation rejects it at registration, the form does not +offer it, enablement requires the default-off custom MCP governance gate (which admits +only `api_key`/`oauth_client_credentials`), and a historical row fails closed at connect. +The existing `oauth_client_credentials` transport consumes a previously stored +bearer token; it does not implement issuance or refresh. External write governance, +AWS-mutation freeze and per-account exposed-tool ceilings are unchanged. + +## Rollout order + +Before strict credential-blind producers roll out, complete administrator backfill +and verification for migrated defaults using +[Datasource credential identity and rollout](../runbooks/datasource-credential-isolation.md). +Deploy the helper and Lambda producers before the Web reader/UI. Roll out the +AgentCore image containing the pinned transport separately. Review a saved +Terraform plan before applying optional private-network settings. No source merge +alone proves activation or vendor readiness. diff --git a/web/app/api/datasources/authz.test.ts b/web/app/api/datasources/authz.test.ts index eb40389f2..29f4a0b9c 100644 --- a/web/app/api/datasources/authz.test.ts +++ b/web/app/api/datasources/authz.test.ts @@ -16,7 +16,7 @@ vi.mock('@/lib/datasources', () => ({ createDatasource: vi.fn(), updateDatasource: vi.fn(), })); vi.mock('@/lib/integration-credentials', () => ({ - getConfiguredIds: vi.fn(async () => []), + getConfiguredDatasourceIds: vi.fn(async () => []), setIntegrationCredentialById: vi.fn(), mirrorDefaultCredential: vi.fn(), })); vi.mock('@/lib/mcp-lambda-invoke', () => ({ invokeMcpLambdaTool: vi.fn(), KNOWN_MCP_LAMBDA_KINDS: ['prometheus'] })); diff --git a/web/app/api/datasources/manage/route.test.ts b/web/app/api/datasources/manage/route.test.ts index 0f75a7df2..63eab3e48 100644 --- a/web/app/api/datasources/manage/route.test.ts +++ b/web/app/api/datasources/manage/route.test.ts @@ -59,6 +59,28 @@ beforeEach(() => { }); describe('POST create', () => { + it('legacy Datadog credentials cannot follow a changed endpoint origin', async () => { + getDatasource.mockResolvedValue({ id: 7, name: 'dd', kind: 'datadog', endpoint: 'https://old.example.test', + authType: 'custom_header', isDefault: true, settings: {} }); + getCredentialById.mockResolvedValue({ endpoint: 'https://old.example.test', authType: 'custom_header', + apiKey: 'synthetic-api', appKey: 'synthetic-app' }); + const { PATCH } = await import('./route'); + expect((await PATCH(req({ id: 7, endpoint: 'https://new.example.test' }, 'PATCH'))).status).toBe(200); + const blob = setIntegrationCredentialById.mock.calls.at(-1)![1]; + expect(blob.endpoint).toBe('https://new.example.test'); + expect(blob).not.toHaveProperty('apiKey'); + expect(blob).not.toHaveProperty('appKey'); + }); + it('a settings-only PATCH keeps a Datadog legacy key pair whatever the row authType says', async () => { + getDatasource.mockResolvedValue({ id: 7, name: 'dd', kind: 'datadog', endpoint: 'https://api.datadoghq.com', + authType: 'none', isDefault: false, settings: {} }); + getCredentialById.mockResolvedValue({ endpoint: 'https://api.datadoghq.com', authType: 'none', + apiKey: 'synthetic-api', appKey: 'synthetic-app' }); + const { PATCH } = await import('./route'); + expect((await PATCH(req({ id: 7, settings: { timeoutS: 20 } }, 'PATCH'))).status).toBe(200); + const blob = setIntegrationCredentialById.mock.calls.at(-1)![1]; + expect(blob).toMatchObject({ apiKey: 'synthetic-api', appKey: 'synthetic-app', timeoutS: 20 }); + }); it('403 non-admin, no writes', async () => { isAdmin.mockResolvedValue(false); const { POST } = await import('./route'); diff --git a/web/app/api/datasources/manage/route.ts b/web/app/api/datasources/manage/route.ts index 8dc33c4e0..a58ffc840 100644 --- a/web/app/api/datasources/manage/route.ts +++ b/web/app/api/datasources/manage/route.ts @@ -22,6 +22,9 @@ const AUTH_TYPES = ['none', 'basic', 'bearer', 'custom_header']; // through the creds spread (round-5: creds.endpoint would otherwise override the validated // endpoint in the blob without tripping the host-change guard). const CRED_KEYS = ['username', 'password', 'token', 'headerName', 'headerValue', 'headerName2', 'headerValue2', 'org_id'] as const; +// Legacy server-side Datadog blobs can contain a dedicated key pair. They are not client +// writable here, but must participate in the same origin/auth-change scrub. +const STORED_AUTH_KEYS = [...CRED_KEYS, 'apiKey', 'appKey'] as const; /** True when a present `settings` value is not a plain object — 400, never a silent clear * (round-8: null/array/string/number shapes bypassed the round-6 empty-sanitize guard). */ function settingsShapeInvalid(body: Record): boolean { @@ -87,6 +90,7 @@ export async function POST(request: Request) { // database is ClickHouse-only config — never persist it on the row for other kinds either // (round-12: the blob-side delete alone left stale, admin-visible row config). if (kind !== 'clickhouse') delete settings.database; + if (kind !== 'mimir') delete settings.apiPrefix; if (body.settings && typeof body.settings === 'object' && !Array.isArray(body.settings) && Object.keys(body.settings as object).length > 0 && Object.keys(settings).length === 0) { return json({ error: 'invalid settings (timeoutS 1–60 integer; database identifier, non-system)' }, 400); @@ -139,6 +143,7 @@ export async function PATCH(request: Request) { if (settingsShapeInvalid(body)) return json({ error: 'settings must be an object' }, 400); const settings = body.settings !== undefined ? sanitizeDsSettings(body.settings) : undefined; if (settings !== undefined && ds.kind !== 'clickhouse') delete settings.database; + if (settings !== undefined && ds.kind !== 'mimir') delete settings.apiPrefix; if (body.settings && typeof body.settings === 'object' && !Array.isArray(body.settings) && Object.keys(body.settings as object).length > 0 && settings !== undefined && Object.keys(settings).length === 0) { return json({ error: 'invalid settings (timeoutS 1–60 integer; database identifier, non-system)' }, 400); @@ -185,6 +190,7 @@ export async function PATCH(request: Request) { delete existing.timeoutS; delete existing.database; delete existing.instanceId; + delete existing.apiPrefix; // ORIGIN compare (scheme+host+port — an https→http downgrade must count as a change, or // Basic material would transmit in cleartext); a malformed URL counts as changed. const originOf = (u: string | null | undefined): string | null => { try { return new URL(u ?? '').origin; } catch { return null; } }; @@ -192,21 +198,24 @@ export async function PATCH(request: Request) { // instance's blob — trust it as a merge base only when its endpoint origin matches THIS // row's; otherwise drop the auth keys rather than bind foreign creds to this endpoint. if (ds.isDefault && existing.endpoint && originOf(String(existing.endpoint)) !== originOf(ds.endpoint)) { - for (const k of CRED_KEYS) delete existing[k]; + for (const k of STORED_AUTH_KEYS) delete existing[k]; } const hostChanged = endpoint !== undefined && originOf(endpoint) !== originOf(ds.endpoint); // On a host change, stored auth material is dropped UNCONDITIONALLY (round-5: a partial // creds object like {username} must not carry the stored password to the new origin) — // whatever the client genuinely re-supplied is reinstated by the creds spread below. if (hostChanged) { - for (const k of CRED_KEYS) delete existing[k]; // org_id included — tenant id is host-scoped + for (const k of STORED_AUTH_KEYS) delete existing[k]; // tenant and legacy keys are origin-scoped } const effAuth = authType ?? ds.authType ?? 'none'; const KEEP_BY_AUTH: Record = { none: [], basic: ['username', 'password'], bearer: ['token'], - custom_header: ['headerName', 'headerValue', 'headerName2', 'headerValue2'], + custom_header: ['headerName', 'headerValue', 'headerName2', 'headerValue2', 'apiKey', 'appKey'], }; - for (const k of CRED_KEYS) if (k !== 'org_id' && !(KEEP_BY_AUTH[effAuth] ?? []).includes(k)) delete existing[k]; + // A Datadog instance's legacy key pair is its credential whatever the row's authType says, so an + // unrelated settings-only PATCH never deletes it (only an origin change or an explicit re-auth does). + const keep = ds.kind === 'datadog' ? [...(KEEP_BY_AUTH[effAuth] ?? []), 'apiKey', 'appKey'] : (KEEP_BY_AUTH[effAuth] ?? []); + for (const k of STORED_AUTH_KEYS) if (k !== 'org_id' && !keep.includes(k)) delete existing[k]; // creds is key-allowlisted (pickCredKeys) so nothing here can override the validated // endpoint/authType/settings fields regardless of spread order. const blob: Record = { @@ -218,7 +227,7 @@ export async function PATCH(request: Request) { }; // Prune the FINAL blob too (round-10: {authType:'none', creds:{password}} re-added the // key AFTER the merge-base pruning) — keys outside the effective authType never persist. - for (const k of CRED_KEYS) if (k !== 'org_id' && !(KEEP_BY_AUTH[effAuth] ?? []).includes(k)) delete blob[k]; + for (const k of STORED_AUTH_KEYS) if (k !== 'org_id' && !keep.includes(k)) delete blob[k]; // database is ClickHouse-only config — never persist it for other kinds (inert but stale). if (ds.kind !== 'clickhouse') delete blob.database; await setIntegrationCredentialById(id, blob, client); diff --git a/web/app/api/datasources/query/route.test.ts b/web/app/api/datasources/query/route.test.ts index 6569f59bf..7e19a1e1b 100644 --- a/web/app/api/datasources/query/route.test.ts +++ b/web/app/api/datasources/query/route.test.ts @@ -94,7 +94,7 @@ describe('POST /api/datasources/query', () => { }); // gap L203: per-datasource timeout setting - it('an instance timeoutS rides prometheus args (capped at 10s under the connector HTTP timeout)', async () => { + it('an instance timeoutS rides prometheus args with the Lambda execution ceiling', async () => { getDatasource.mockResolvedValue({ id: 3, kind: 'prometheus', settings: { timeoutS: 5 } }); resolveConnConfig.mockResolvedValue({ endpoint: 'http://prom.internal:9090' }); invokeMcpLambdaTool.mockResolvedValue({ resultType: 'vector', result: [] }); @@ -103,7 +103,7 @@ describe('POST /api/datasources/query', () => { expect(invokeMcpLambdaTool.mock.calls.at(-1)![0].args.timeout).toBe('5s'); getDatasource.mockResolvedValue({ id: 3, kind: 'prometheus', settings: { timeoutS: 60 } }); await POST(req({ id: 3, query: 'up' })); - expect(invokeMcpLambdaTool.mock.calls.at(-1)![0].args.timeout).toBe('10s'); // capped + expect(invokeMcpLambdaTool.mock.calls.at(-1)![0].args.timeout).toBe('55s'); }); it('clickhouse sends NO timeout arg — the bound rides connConfig (the connector defaults/clamps it)', async () => { @@ -201,12 +201,12 @@ describe('POST /api/datasources/query', () => { expect(invokeMcpLambdaTool.mock.calls.at(-1)![0].tool).toBe('prometheus_query'); }); - it('legacy range:true → range tool with NO start/step args (1h connector default)', async () => { + it('legacy range:true retains the one-hour window and default step', async () => { const { POST } = await import('./route'); await POST(req({ slug: 'prometheus', query: 'up', range: true })); const call = invokeMcpLambdaTool.mock.calls.at(-1)![0]; expect(call.tool).toBe('prometheus_query_range'); - expect(call.args.start).toBeUndefined(); + expect(Number(call.args.end) - Number(call.args.start)).toBe(3600); expect(call.args.step).toBeUndefined(); }); }); diff --git a/web/app/api/datasources/query/route.ts b/web/app/api/datasources/query/route.ts index 07d17ec6e..06c9ffc79 100644 --- a/web/app/api/datasources/query/route.ts +++ b/web/app/api/datasources/query/route.ts @@ -10,7 +10,7 @@ import { isDatasourceKind } from '@/lib/integrations-category'; import { assertDatasourceEndpointAllowed } from '@/lib/ssrf-guard'; import { normalizeResult } from '@/lib/datasource-render'; import { readJsonBounded, BodyTooLargeError } from '@/lib/http-body'; -import { TOOL } from '@/lib/datasource-query-tools'; +import { TOOL, EFFECTIVE_TIMEOUT_CAP, EXECUTION_TIMEOUT_KINDS } from '@/lib/datasource-query-tools'; export const dynamic = 'force-dynamic'; @@ -30,12 +30,18 @@ export async function POST(request: Request) { if (e instanceof BodyTooLargeError) return json({ error: 'request body too large' }, 413); return json({ error: 'invalid JSON body' }, 400); } + if (!body || typeof body !== 'object' || Array.isArray(body)) return json({ error: 'body must be an object' }, 400); // Resolve the kind + (for an instance id) the inline conn-config. let kind = ''; let connConfig: ConnConfig | undefined; let dsTimeoutS: number | undefined; // gap L203: per-datasource upstream execution bound const id = Number(body.id); + if (body.id !== undefined && ( + !['number', 'string'].includes(typeof body.id) + || typeof body.id === 'string' && !/^[1-9]\d*$/.test(body.id) + || !Number.isSafeInteger(id) || id <= 0 + )) return json({ error: 'invalid datasource instance id' }, 400); if (Number.isInteger(id) && id > 0) { const ds = await getDatasource(id); if (!ds || !isDatasourceKind(ds.kind)) return json({ error: 'unknown datasource instance' }, 400); @@ -60,22 +66,20 @@ export async function POST(request: Request) { const args: Record = { [spec.arg]: query, ...(spec.extra ?? {}) }; - // Upstream execution bound (review hardening + gap L203 per-datasource setting): - // prometheus/mimir accept a `timeout` API param — the effective value is the datasource's - // own timeoutS (validated 1..60) further capped at 10s so it stays UNDER the connector's - // 12s HTTP timeout (a longer upstream bound than the HTTP client's is dead config). - // clickhouse needs NO arg here: its timeoutS rides the conn config (resolveConnConfig) and - // the connector applies it as the default max_execution_time on EVERY path — Explore, - // service-graph sources, and the agent/worker secret path — aligning its own HTTP timeout - // above the bound. Other kinds see no unknown arg. - if (kind === 'prometheus' || kind === 'mimir') args.timeout = `${Math.min(dsTimeoutS ?? 10, 10)}s`; + // Provider execution ceilings belong to the connector, on every invocation path. + // Keep the argument as an additional ceiling for older deployed PromQL producers. + if (kind === 'prometheus' || kind === 'mimir') args.timeout = `${Math.min(dsTimeoutS ?? 10, EFFECTIVE_TIMEOUT_CAP)}s`; - // Range mode: absent/false = instant; true = legacy 1h range (connector default); - // { window, step } = explicit time range. An object range is validated regardless of kind (so a bad - // window/step is a 400, not a silent instant); start/end are computed from the request clock. + // Search windows are independent of whether the provider exposes a separate range tool. let tool = spec.instant; const r = body.range; - if (r && typeof r === 'object') { + let queriedWindow: { start: string; end: string } | null = null; + const nowSec = Math.floor(Date.now() / 1000); + if (r !== undefined && r !== false && r !== true && (!r || typeof r !== 'object' || Array.isArray(r))) { + return json({ error: 'range must be false, true, or a window object' }, 400); + } + if (r && !spec.range && !spec.timeWindow) return json({ error: 'this datasource requires time filters in its query' }, 400); + if (r && typeof r === 'object' && !Array.isArray(r)) { const window = Number((r as { window?: unknown }).window); const step = Number((r as { step?: unknown }).step); // Upper bound widened to 30d (gap-audit L86, v1 parity) — the point-density cap below still @@ -83,10 +87,8 @@ export async function POST(request: Request) { // `timeout` param (range + instant) and, for Loki, by the tighter 7d window below (Loki accepts // no per-request timeout). // A 30d window at the UI's ~250-point autoStep is ~10368s steps. - // Per-kind upper bound (review): prometheus/mimir get 30d WITH a forwarded upstream `timeout` - // (their connectors clamp it 1..60s on BOTH instant and range paths); Loki has no per-request - // upstream timeout, so its widened bound stays at 7d. - const maxWindow = kind === 'loki' ? 604800 : 2592000; + // Metric queries permit 30d; log/trace searches retain the narrower 7d bound. + const maxWindow = ['loki', 'tempo', 'jaeger'].includes(kind) ? 604800 : 2592000; if (!Number.isInteger(window) || window < 60 || window > maxWindow) { return json({ error: `range.window must be an integer in [60, ${maxWindow}] seconds` }, 400); } @@ -99,22 +101,30 @@ export async function POST(request: Request) { if (Math.ceil(window / step) > 5000) { return json({ error: 'range too dense: ceil(window / step) must be ≤ 5000 points' }, 400); } - if (spec.range) { // kind has a range tool; otherwise the validated window is ignored (instant) - const nowSec = Math.floor(Date.now() / 1000); + args.start = String(nowSec - window); + args.end = String(nowSec); + queriedWindow = { start: args.start as string, end: args.end as string }; + if (spec.range) { tool = spec.range; - args.start = String(nowSec - window); - args.end = String(nowSec); args.step = String(step); } - } else if (r === true && spec.range) { - tool = spec.range; // back-compat: connector applies its 1h / 60s default + } else if (r === true || spec.timeWindow) { + if (spec.range) tool = spec.range; + args.start = String(nowSec - 3600); + args.end = String(nowSec); + queriedWindow = { start: args.start as string, end: args.end as string }; } try { const t0 = Date.now(); const result = await invokeMcpLambdaTool({ kind, tool, args, connConfig }); // Additive metadata for the Explore result bar (gap-audit L88) — existing consumers read only `result`. - return json({ result: normalizeResult(kind, tool, result), metadata: { executionTimeMs: Date.now() - t0, tool } }, 200); + const timeoutMode = EXECUTION_TIMEOUT_KINDS.has(kind) ? 'execution' : 'transport'; + return json({ result: normalizeResult(kind, tool, result), metadata: { + executionTimeMs: Date.now() - t0, tool, window: queriedWindow, scope: 'shared', + // A slug-only legacy call has no row settings available to the BFF. + ...(connConfig ? { timeoutMode, effectiveTimeoutS: Math.min(dsTimeoutS ?? (timeoutMode === 'execution' ? 10 : 12), EFFECTIVE_TIMEOUT_CAP) } : {}), + } }, 200); } catch (e) { return json({ error: e instanceof Error ? e.message : 'query failed' }, 502); } diff --git a/web/app/api/datasources/query/window.test.ts b/web/app/api/datasources/query/window.test.ts new file mode 100644 index 000000000..0725c24e2 --- /dev/null +++ b/web/app/api/datasources/query/window.test.ts @@ -0,0 +1,70 @@ +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const invoke = vi.fn(); +const getDatasource = vi.fn(); +vi.mock('@/lib/auth', () => ({ verifyUser: async () => ({ sub: 'synthetic-user' }) })); +vi.mock('@/lib/mcp-lambda-invoke', () => ({ invokeMcpLambdaTool: (...args: unknown[]) => invoke(...args) })); +vi.mock('@/lib/datasources', () => ({ + getDatasource: (...args: unknown[]) => getDatasource(...args), + resolveConnConfig: async () => ({ endpoint: 'https://provider.example.test', timeoutS: 35 }), +})); +import { POST } from './route'; + +function request(kind: string, range?: unknown, id?: number) { + return new Request('https://app.example.test/api/datasources/query', { + method: 'POST', headers: { 'content-type': 'application/json' }, + body: JSON.stringify({ slug: kind, query: 'synthetic', range, id }), + }); +} + +beforeEach(() => { + invoke.mockReset().mockResolvedValue({ result: [], traces: [], series: [] }); + getDatasource.mockReset(); +}); + +describe('provider query-window contract', () => { + it.each(['tempo', 'jaeger', 'dynatrace', 'datadog'])('forwards a selected window to %s and returns the effective bounds', async kind => { + const response = await POST(request(kind, { window: 86400, step: 360 })); + expect(response.status).toBe(200); + const call = invoke.mock.calls[0][0]; + expect(Number(call.args.end) - Number(call.args.start)).toBe(86400); + expect(call.args.step).toBeUndefined(); + const body = await response.json(); + expect(body.metadata.window).toEqual({ start: call.args.start, end: call.args.end }); + expect(body.metadata.scope).toBe('shared'); + }); + + it.each(['tempo', 'jaeger', 'dynatrace', 'datadog'])('%s explicitly retains its one-hour default without a selected range', async kind => { + await POST(request(kind)); + const args = invoke.mock.calls[0][0].args; + expect(Number(args.end) - Number(args.start)).toBe(3600); + }); + + it.each([true, { window: 3600, step: 60 }])('rejects a SQL API range instead of ignoring it', async range => { + expect((await POST(request('clickhouse', range))).status).toBe(400); + expect(invoke).not.toHaveBeenCalled(); + }); + + it.each([[], '24h', 42, null])('rejects malformed range shapes', async range => { + expect((await POST(request('tempo', range))).status).toBe(400); + expect(invoke).not.toHaveBeenCalled(); + }); + + it('returns the effective provider or transport ceiling without exposing connection credentials', async () => { + getDatasource.mockResolvedValue({ id: 1, kind: 'prometheus', settings: { timeoutS: 35 } }); + const response = await POST(request('prometheus', false, 1)); + const body = await response.json(); + expect(invoke.mock.calls[0][0].args.timeout).toBe('35s'); + expect(body.metadata).toMatchObject({ timeoutMode: 'execution', effectiveTimeoutS: 35 }); + expect(JSON.stringify(body)).not.toContain('provider.example.test'); + }); + + it.each([false, true, 0, -1, 'missing', '1.5', Number.MAX_SAFE_INTEGER + 1])('a malformed explicit ID cannot select the legacy kind default', async id => { + const response = await POST(new Request('https://app.example.test/api/datasources/query', { + method: 'POST', body: JSON.stringify({ id, slug: 'prometheus', query: 'up' }), + })); + expect(response.status).toBe(400); + expect(invoke).not.toHaveBeenCalled(); + expect(getDatasource).not.toHaveBeenCalled(); + }); +}); diff --git a/web/app/api/datasources/route.test.ts b/web/app/api/datasources/route.test.ts index 6590f67bd..87a03b22a 100644 --- a/web/app/api/datasources/route.test.ts +++ b/web/app/api/datasources/route.test.ts @@ -3,7 +3,7 @@ import { describe, it, expect, vi, beforeEach } from 'vitest'; const verifyUser = vi.fn(); const isAdmin = vi.fn(); const listDatasources = vi.fn(); -const getConfiguredIds = vi.fn(); +const getConfiguredDatasourceIds = vi.fn(); const deleteDatasource = vi.fn(); vi.mock('@/lib/auth', () => ({ verifyUser: (...a: unknown[]) => verifyUser(...a) })); vi.mock('@/lib/admin', () => ({ isAdmin: (...a: unknown[]) => isAdmin(...a) })); @@ -11,20 +11,20 @@ vi.mock('@/lib/datasources', () => ({ listDatasources: (...a: unknown[]) => listDatasources(...a), deleteDatasource: (...a: unknown[]) => deleteDatasource(...a), })); -vi.mock('@/lib/integration-credentials', () => ({ getConfiguredIds: (...a: unknown[]) => getConfiguredIds(...a) })); +vi.mock('@/lib/integration-credentials', () => ({ getConfiguredDatasourceIds: (...a: unknown[]) => getConfiguredDatasourceIds(...a) })); const get = () => new Request('http://x/api/datasources', { headers: { cookie: 'awsops_token=t' } }); const del = () => new Request('http://x/api/datasources/5', { method: 'DELETE', headers: { cookie: 'awsops_token=t' } }); beforeEach(() => { - for (const m of [verifyUser, isAdmin, listDatasources, getConfiguredIds, deleteDatasource]) m.mockReset(); + for (const m of [verifyUser, isAdmin, listDatasources, getConfiguredDatasourceIds, deleteDatasource]) m.mockReset(); verifyUser.mockResolvedValue({ sub: 'u' }); isAdmin.mockResolvedValue(true); listDatasources.mockResolvedValue([ { id: 1, name: 'prod-prom', kind: 'prometheus', endpoint: 'http://p', authType: 'none', isDefault: true, enabled: true }, { id: 2, name: 'stg-prom', kind: 'prometheus', endpoint: 'http://s', authType: 'basic', isDefault: false, enabled: true }, ]); - getConfiguredIds.mockResolvedValue(['2']); + getConfiguredDatasourceIds.mockResolvedValue(['1', '2']); deleteDatasource.mockResolvedValue(undefined); }); @@ -41,7 +41,8 @@ describe('GET /api/datasources (list instances)', () => { const { datasources } = await resp.json(); expect(datasources).toHaveLength(2); // admin (default mock): the registered URL is included (v1 parity for managers) - expect(datasources[0]).toEqual({ id: 1, name: 'prod-prom', kind: 'prometheus', endpoint: 'http://p', authType: 'none', isDefault: true, connected: true }); + expect(datasources[0]).toEqual({ id: 1, name: 'prod-prom', kind: 'prometheus', endpoint: 'http://p', authType: 'none', isDefault: true, + connected: true, configured: true, connectionStatus: 'not_checked', scope: 'shared' }); expect(datasources[1].connected).toBe(true); // id '2' has a credential }); @@ -59,6 +60,14 @@ describe('GET /api/datasources (list instances)', () => { expect(resp.status).toBe(200); expect((await resp.json()).datasources).toEqual([]); }); + it('does not infer a default instance is configured or connected without its auth configuration', async () => { + listDatasources.mockResolvedValue([{ id: 1, name: 'default-missing', kind: 'prometheus', + endpoint: 'https://provider.example.test', authType: 'bearer', isDefault: true }]); + getConfiguredDatasourceIds.mockResolvedValue([]); + const { GET } = await import('./route'); + const { datasources } = await (await GET(get())).json(); + expect(datasources[0]).toMatchObject({ isDefault: true, configured: false, connected: false, connectionStatus: 'not_checked' }); + }); }); describe('DELETE /api/datasources/[id]', () => { diff --git a/web/app/api/datasources/route.ts b/web/app/api/datasources/route.ts index 35bc2cfaa..dae50625c 100644 --- a/web/app/api/datasources/route.ts +++ b/web/app/api/datasources/route.ts @@ -1,10 +1,11 @@ // GET /api/datasources — list configured datasource INSTANCES for the hub + Explore picker. // Authenticated (read-only), NOT admin. Returns {id,name,kind,authType,isDefault,connected} — never -// credentials. Degrade-safe: [] when Aurora is off / on error so the page doesn't 500. +// credentials. Configured is distinct from tested connectivity; connected is a legacy +// configuration alias. Degrade-safe: [] when Aurora is off / on error. import { verifyUser } from '@/lib/auth'; import { listDatasources } from '@/lib/datasources'; import { isAdmin } from '@/lib/admin'; -import { getConfiguredIds } from '@/lib/integration-credentials'; +import { getConfiguredDatasourceIds } from '@/lib/integration-credentials'; export const dynamic = 'force-dynamic'; @@ -17,7 +18,8 @@ export async function GET(request: Request) { if (!user) return json({ error: 'unauthenticated' }, 401); try { - const [rows, configuredIds, admin] = await Promise.all([listDatasources(), getConfiguredIds(), isAdmin(user)]); + const [rows, admin] = await Promise.all([listDatasources(), isAdmin(user)]); + const configuredIds = await getConfiguredDatasourceIds(rows); const idSet = new Set(configuredIds); const datasources = rows.map((r) => ({ id: r.id, @@ -28,8 +30,10 @@ export async function GET(request: Request) { ...(admin ? { endpoint: r.endpoint, settings: r.settings } : {}), authType: r.authType, isDefault: r.isDefault, - // "connected" = a credential is resolvable: the instance id key, or (for migrated defaults) the kind mirror. - connected: idSet.has(String(r.id)) || r.isDefault, + configured: idSet.has(String(r.id)), + connected: idSet.has(String(r.id)), // compatibility alias; not a live connection test + connectionStatus: 'not_checked', + scope: 'shared', })); return json({ datasources }, 200); } catch { diff --git a/web/app/api/datasources/test/route.test.ts b/web/app/api/datasources/test/route.test.ts index 1b5fe782b..6819cd736 100644 --- a/web/app/api/datasources/test/route.test.ts +++ b/web/app/api/datasources/test/route.test.ts @@ -26,6 +26,18 @@ beforeEach(() => { }); describe('POST /api/datasources/test', () => { + it('candidate credentials cannot replace the validated endpoint or auth method', async () => { + const { POST } = await import('./route'); + const response = await POST(req({ + kind: 'mimir', endpoint: 'https://provider.example.test', authType: 'none', + creds: { endpoint: 'http://169.254.169.254/', authType: 'basic', timeoutS: 999 }, + settings: { apiPrefix: '/metrics', timeoutS: 35 }, + })); + expect(response.status).toBe(200); + expect(invokeMcpLambdaTool.mock.calls[0][0].connConfig).toEqual({ + endpoint: 'https://provider.example.test', authType: 'none', apiPrefix: '/metrics', timeoutS: 35, + }); + }); it('401 unauthenticated / 403 non-admin', async () => { verifyUser.mockResolvedValueOnce(null); let { POST } = await import('./route'); @@ -41,7 +53,7 @@ describe('POST /api/datasources/test', () => { const { POST } = await import('./route'); const resp = await POST(req({ kind: 'prometheus', endpoint: 'http://p:9090', authType: 'bearer', creds: { token: 't' } })); expect(resp.status).toBe(200); - expect(await resp.json()).toEqual({ ok: true, latencyMs: 42, error: undefined }); + expect(await resp.json()).toEqual({ ok: true, latencyMs: 42, verified: [] }); const call = invokeMcpLambdaTool.mock.calls[0][0]; expect(call.tool).toBe('prometheus_health'); expect(call.connConfig).toEqual({ endpoint: 'http://p:9090', authType: 'bearer', token: 't' }); diff --git a/web/app/api/datasources/test/route.ts b/web/app/api/datasources/test/route.ts index dc4978260..2f03614a6 100644 --- a/web/app/api/datasources/test/route.ts +++ b/web/app/api/datasources/test/route.ts @@ -1,13 +1,17 @@ // POST /api/datasources/test — probe a datasource connection BEFORE saving (v1 "Test connection"). // Admin-gated. Takes an UNSAVED candidate {kind, endpoint, authType, creds}, SSRF-guards the endpoint, // and invokes the connector's `${kind}_health` tool with an INLINE conn-config (nothing is stored). -// SECURITY: the credential value is never logged or echoed — only {ok, latency_ms, error?} is returned. +// The health tool runs ONE real bounded query (not a readiness endpoint — an unauthenticated /ready +// used to pass), so `verified: ['query']` is query-access proof. +// SECURITY: the credential value is never logged or echoed — only {ok, latencyMs, verified, error?} +// is returned, with fixed connector sentences and never upstream text. import { verifyUser } from '@/lib/auth'; import { isAdmin } from '@/lib/admin'; import { invokeMcpLambdaTool, KNOWN_MCP_LAMBDA_KINDS } from '@/lib/mcp-lambda-invoke'; import { isDatasourceKind } from '@/lib/integrations-category'; import { assertDatasourceEndpointAllowed } from '@/lib/ssrf-guard'; import { readJsonBounded, BodyTooLargeError } from '@/lib/http-body'; +import { sanitizeDsSettings } from '@/lib/datasources'; export const dynamic = 'force-dynamic'; @@ -20,7 +24,7 @@ export async function POST(request: Request) { if (!user) return json({ error: 'unauthenticated' }, 401); if (!(await isAdmin(user))) return json({ error: 'admin access required' }, 403); - let body: { kind?: unknown; endpoint?: unknown; authType?: unknown; creds?: unknown }; + let body: { kind?: unknown; endpoint?: unknown; authType?: unknown; creds?: unknown; settings?: unknown }; try { body = (await readJsonBounded(request)) as typeof body; } catch (e) { if (e instanceof BodyTooLargeError) return json({ error: 'request body too large' }, 413); @@ -37,15 +41,26 @@ export async function POST(request: Request) { catch (e) { return json({ error: (e as Error).message }, 400); } const authType = typeof body.authType === 'string' ? body.authType : 'none'; + if (!['none', 'basic', 'bearer', 'custom_header'].includes(authType)) return json({ error: 'unknown auth method' }, 400); const creds = (body.creds && typeof body.creds === 'object' && !Array.isArray(body.creds)) ? (body.creds as Record) : {}; - const connConfig = { endpoint, authType, ...creds }; // flat blob the connector Lambda understands + const credentialFields = Object.fromEntries( + ['username', 'password', 'token', 'headerName', 'headerValue', 'headerName2', 'headerValue2', 'org_id'] + .filter(key => typeof creds[key] === 'string').map(key => [key, creds[key]]), + ); + const settings = sanitizeDsSettings(body.settings); + if (kind !== 'clickhouse') delete settings.database; + if (kind !== 'mimir') delete settings.apiPrefix; + const connConfig = { ...credentialFields, ...settings, endpoint, authType }; try { + const started = Date.now(); const result = (await invokeMcpLambdaTool({ kind, tool: `${kind}_health`, connConfig, - })) as { ok?: boolean; latency_ms?: number; error?: string }; - return json({ ok: Boolean(result?.ok), latencyMs: result?.latency_ms, error: result?.error }, 200); + })) as { ok?: boolean; latency_ms?: number; error?: string; verified?: unknown }; + return json({ ok: result?.ok === true, latencyMs: result?.latency_ms ?? Date.now() - started, error: result?.error, + verified: Array.isArray(result?.verified) ? result.verified.filter(value => value === 'query') : [], + }, 200); } catch (e) { // message only — connector errors never carry the credential value return json({ ok: false, error: e instanceof Error ? e.message : 'connection test failed' }, 200); diff --git a/web/app/customization/page.tsx b/web/app/customization/page.tsx index 73cf83eaa..1fdb7d9d1 100644 --- a/web/app/customization/page.tsx +++ b/web/app/customization/page.tsx @@ -19,7 +19,9 @@ const AGENT_TYPES = ['generic', 'on_demand', 'triage', 'rca', 'mitigation', 'eva // jaeger/dynatrace) — see web/lib/integration-validation.ts for the source of truth. const INTEG_KINDS_EGRESS = INTEGRATION_KINDS_EGRESS; const INTEG_KINDS_INGRESS = INTEGRATION_KINDS_INGRESS; -const INTEG_TRANSPORTS = INTEGRATION_TRANSPORTS; +// oauth_3lo stays in the source-of-truth constant for historical rows but is not executable by the +// runtime and is rejected by validation, so the form never offers it. +const INTEG_TRANSPORTS = INTEGRATION_TRANSPORTS.filter((t) => t !== 'oauth_3lo'); // NOTE: curated read connectors (Prometheus/Loki/…/Notion credential cards) moved to the Integrations // hub (/integrations) — Datasources tab + Connectors tab. This page keeps Agents/Skills/Agent-Space + // the advanced custom-integration registration. diff --git a/web/app/integrations/datasources/DatasourceForm.tsx b/web/app/integrations/datasources/DatasourceForm.tsx index 937bb5ccc..9f4d94ba7 100644 --- a/web/app/integrations/datasources/DatasourceForm.tsx +++ b/web/app/integrations/datasources/DatasourceForm.tsx @@ -4,6 +4,7 @@ import Input from '@/components/ui/Input'; import Button from '@/components/ui/Button'; import IntegrationIcon from '@/components/datasources/IntegrationIcon'; import { useI18n } from '@/components/shell/LanguageProvider'; +import { EXECUTION_TIMEOUT_KINDS, EFFECTIVE_TIMEOUT_CAP } from '@/lib/datasource-query-tools'; // v1-parity Add/Edit form: multiple instances per type, a name, an explicit auth method (optional auth), // and a Test-before-save probe. POSTs (create) / PATCHes (update) /api/datasources/manage. @@ -39,7 +40,7 @@ export interface DatasourceFormValue { authType: string; isDefault?: boolean; // gap L203: per-datasource connection settings (server-side sanitized; see lib/datasources.ts) - settings?: { timeoutS?: number; database?: string }; + settings?: { timeoutS?: number; database?: string; apiPrefix?: string }; } export default function DatasourceForm({ @@ -55,6 +56,7 @@ export default function DatasourceForm({ // gap L203: settings kept as strings for the inputs; settingsPayload() validates/coerces const [timeoutS, setTimeoutS] = useState(initial?.settings?.timeoutS != null ? String(initial.settings.timeoutS) : ''); const [database, setDatabase] = useState(initial?.settings?.database ?? ''); + const [apiPrefix, setApiPrefix] = useState(initial?.settings?.apiPrefix ?? '/prometheus'); const [test, setTest] = useState<{ ok: boolean; ms?: number; error?: string } | null>(null); const [testing, setTesting] = useState(false); const [saving, setSaving] = useState(false); @@ -77,11 +79,15 @@ export default function DatasourceForm({ const databaseInvalid = kind === 'clickhouse' && dbTrim !== '' && (!/^[A-Za-z_][A-Za-z0-9_]*$/.test(dbTrim) || dbTrim.length > 128 || ['system', 'information_schema'].includes(dbTrim.toLowerCase())); + const prefixInvalid = kind === 'mimir' && (apiPrefix.length > 128 || !/^(?:\/[A-Za-z0-9_-]+)*$/.test(apiPrefix)); + const executionTimeout = EXECUTION_TIMEOUT_KINDS.has(kind); + const effectiveTimeout = Math.min(timeoutS.trim() && !timeoutInvalid ? Number(timeoutS) : executionTimeout ? 10 : 12, EFFECTIVE_TIMEOUT_CAP); const settingsPayload = () => { - const out: { timeoutS?: number; database?: string } = {}; + const out: { timeoutS?: number; database?: string; apiPrefix?: string } = {}; const t = Number(timeoutS); if (timeoutS.trim() !== '' && Number.isInteger(t) && t >= 1 && t <= 60) out.timeoutS = t; if (kind === 'clickhouse' && database.trim()) out.database = database.trim(); + if (kind === 'mimir') out.apiPrefix = apiPrefix; return out; }; @@ -90,7 +96,7 @@ export default function DatasourceForm({ try { const r = await fetch('/api/datasources/test', { method: 'POST', headers: { 'content-type': 'application/json' }, - body: JSON.stringify({ kind, endpoint, authType, creds: credPayload() }), + body: JSON.stringify({ kind, endpoint, authType, creds: credPayload(), settings: settingsPayload() }), }); const b = await r.json(); if (!r.ok) { setErr(b.error || tt(`오류 ${r.status}`)); return; } @@ -138,6 +144,7 @@ export default function DatasourceForm({
setEndpoint(e.target.value)} placeholder={ENDPOINT_PH[kind] ?? 'http://prometheus.internal:9090'} /> +

{tt('사설 주소는 운영자가 네트워크 연결과 DNS를 구성해야 사용할 수 있습니다.')}

@@ -178,8 +185,9 @@ export default function DatasourceForm({ result-cache TTL is deliberately not ported: the v2 query path is uncached by design) */}
- - setTimeoutS(e.target.value)} placeholder={tt('기본 10')} inputMode="numeric" /> + + setTimeoutS(e.target.value)} placeholder={tt(executionTimeout ? '기본 10' : '기본 12')} inputMode="numeric" /> +

{tt('적용 제한')}: {effectiveTimeout}s{!executionTimeout && ` · ${tt('서버 쿼리 중단을 보장하지 않습니다.')}`}

{timeoutInvalid &&

{tt('1–60 사이의 정수를 입력하세요.')}

}
{kind === 'clickhouse' && ( @@ -189,10 +197,18 @@ export default function DatasourceForm({ {databaseInvalid &&

{tt('영문/숫자/밑줄 식별자만 가능하며 system 계열은 사용할 수 없습니다.')}

}
)} + {kind === 'mimir' && ( +
+ + setApiPrefix(e.target.value)} placeholder="/prometheus" /> +

{tt('기본 /prometheus · API가 루트에 있으면 비워 두세요.')}

+ {prefixInvalid &&

{tt('슬래시와 영문/숫자/밑줄/하이픈 경로만 사용할 수 있습니다.')}

} +
+ )}
- {test && ( @@ -205,7 +221,7 @@ export default function DatasourceForm({ {err &&

{err}

}
- +
diff --git a/web/app/integrations/datasources/DatasourcesTab.test.tsx b/web/app/integrations/datasources/DatasourcesTab.test.tsx index e7e4edeaf..dde63d31b 100644 --- a/web/app/integrations/datasources/DatasourcesTab.test.tsx +++ b/web/app/integrations/datasources/DatasourcesTab.test.tsx @@ -17,6 +17,17 @@ beforeEach(() => { afterEach(() => { cleanup(); vi.restoreAllMocks(); }); describe('DatasourcesTab', () => { + it('uses explicit configuration evidence instead of a legacy connected flag', async () => { + global.fetch = vi.fn(async () => ({ + ok: true, status: 200, json: async () => ({ datasources: [{ + id: 1, name: 'unconfirmed-default', kind: 'prometheus', isDefault: true, + connected: true, configured: false, + }] }), + })) as unknown as typeof fetch; + render(); + expect(await screen.findByText('설정 미확인')).toBeTruthy(); + expect(screen.queryByText('● 연결됨')).toBeNull(); + }); it('lists instances (name/type/auth/default) with an Explore link per row', async () => { render(); await waitFor(() => expect(screen.getByText('prod-prom')).toBeTruthy()); @@ -48,7 +59,8 @@ describe('DatasourcesTab', () => { render(); await waitFor(() => expect(screen.getByText('prod-prom')).toBeTruthy()); expect(screen.getByText('총 데이터소스')).toBeTruthy(); - expect(screen.getByText('연결됨')).toBeTruthy(); + expect(screen.getAllByText('설정됨').length).toBeGreaterThan(0); + expect(screen.queryByText('● 연결됨')).toBeNull(); expect(screen.getByText('타입 종류')).toBeTruthy(); expect(screen.getByText('기본 데이터소스')).toBeTruthy(); // single default in this scenario → the tile names it diff --git a/web/app/integrations/datasources/DatasourcesTab.tsx b/web/app/integrations/datasources/DatasourcesTab.tsx index ec36bcada..085912e8d 100644 --- a/web/app/integrations/datasources/DatasourcesTab.tsx +++ b/web/app/integrations/datasources/DatasourcesTab.tsx @@ -10,7 +10,7 @@ import DatasourceForm, { type DatasourceFormValue } from './DatasourceForm'; import { useI18n } from '@/components/shell/LanguageProvider'; interface Instance { - id: number; name: string; kind: string; endpoint?: string | null; authType?: string | null; isDefault?: boolean; connected?: boolean; settings?: { timeoutS?: number; database?: string }; + id: number; name: string; kind: string; endpoint?: string | null; authType?: string | null; isDefault?: boolean; connected?: boolean; configured?: boolean; settings?: { timeoutS?: number; database?: string; apiPrefix?: string }; } // Chat section per datasource kind (the deep-link prompt pins it with a leading /section — @@ -65,7 +65,8 @@ export default function DatasourcesTab({ canManage = false }: { canManage?: bool } // gap-audit L201: KPI roll-up from the already-fetched list — no extra API call. - const connectedN = list.filter((i) => i.connected).length; + const configured = (i: Instance) => i.configured ?? i.connected ?? false; + const configuredN = list.filter(configured).length; const kindN = new Set(list.map((i) => i.kind)).size; // Defaults are PER KIND — one arbitrary name would misrepresent the set, so show the count // (with every kind: name pair in a tooltip) once there's more than one. @@ -76,7 +77,8 @@ export default function DatasourcesTab({ canManage = false }: { canManage?: bool return (
-

{tt('관측성 데이터소스 (Prometheus·Mimir·Loki·Tempo·ClickHouse). 같은 타입 여러 개 등록 가능.')}

+

{tt('관측성 데이터소스 (Prometheus·Mimir·Loki·Tempo·ClickHouse). 같은 타입 여러 개 등록 가능.')} + {' '}{tt('설정 상태는 실제 연결 성공을 의미하지 않습니다.')}

{/* gap-audit L202: re-fetch without a full page reload. */} }
+

{tt('기본 데이터소스는 타입별 공유 연결이며 AWS 계정 선택으로 바뀌지 않습니다.')}

- +
`${d.kind}: ${d.name}`).join(', ')}> @@ -128,7 +131,7 @@ export default function DatasourcesTab({ canManage = false }: { canManage?: bool )} {i.authType ?? 'none'} - {i.connected ? tt('● 연결됨') : tt('○ 미설정')} + {configured(i) ? tt('설정됨') : tt('설정 미확인')} {i.isDefault ? {tt('★ 기본')} : (canManage && )} diff --git a/web/components/datasources/ExplorePanel.tsx b/web/components/datasources/ExplorePanel.tsx index f112d8eab..4abdc95c5 100644 --- a/web/components/datasources/ExplorePanel.tsx +++ b/web/components/datasources/ExplorePanel.tsx @@ -15,6 +15,7 @@ import HBarList from '@/components/charts/HBarList'; import type { NormalizedResult } from '@/lib/datasource-render'; import { cn } from '@/lib/cn'; import { useI18n } from '@/components/shell/LanguageProvider'; +import { TIME_WINDOW_KINDS, TOOL } from '@/lib/datasource-query-tools'; // A datasource INSTANCE (the hub model): identified by bigint id, with a user-given name. export interface DatasourceInstance { @@ -36,7 +37,7 @@ const PH: Record = { dynatrace: 'metricSelector… 예: builtin:host.cpu.usage:avg', datadog: '메트릭 쿼리… 예: avg:system.cpu.user{*}', }; -const RANGE_KINDS = new Set(['prometheus', 'mimir', 'loki']); // kinds with a *_query_range tool +const RANGE_KINDS = new Set(TIME_WINDOW_KINDS); const selectCls = 'rounded-md border border-ink-200 bg-card px-2.5 py-1.5 text-[13px] text-ink-700'; // Explore range presets: label → window seconds (0 = instant snapshot). Step auto-derived for ~250 points. @@ -76,7 +77,10 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) { const ds = list.find((d) => d.id === selId) ?? null; const canRange = ds ? RANGE_KINDS.has(ds.kind) : false; // Loki has no per-request upstream timeout — the API caps it at 7d, so don't offer 30d. - const presets = ds?.kind === 'loki' ? RANGE_PRESETS.filter(([, s]) => s <= 604800) : RANGE_PRESETS; + const searchWindow = ds ? TOOL[ds.kind]?.timeWindow === true : false; + const effectiveRangeWindow = searchWindow ? rangeWindow || 3600 : rangeWindow; + const presets = RANGE_PRESETS.filter(([, seconds]) => + (!searchWindow || seconds > 0) && (!ds || !['loki', 'tempo', 'jaeger'].includes(ds.kind) || seconds <= 604800)); const queryIsMultiline = ds?.kind === 'clickhouse'; useEffect(() => { @@ -98,7 +102,7 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) { const run = useCallback(async (windowOverride?: number, queryOverride?: string) => { const q = queryOverride ?? query; // a quick-query chip can run its expr without waiting on setQuery if (selId === '' || !q.trim()) return; - const w = windowOverride ?? rangeWindow; + const w = windowOverride ?? effectiveRangeWindow; const kindNow = list.find((d) => d.id === selId)?.kind; // bind kind to THIS query, not the live selection const range = canRange && w > 0 ? { window: w, step: autoStep(w, kindNow) } : false; const queriedKind = kindNow; @@ -116,7 +120,7 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) { } catch (e) { setErr(e instanceof Error ? e.message : tt('쿼리 실패')); } finally { setBusy(false); } - }, [selId, query, rangeWindow, canRange, list, tt]); + }, [selId, query, effectiveRangeWindow, canRange, list, tt]); // NL → query (AI drafts, user reviews, then runs). Never auto-runs. const generate = useCallback(async () => { @@ -142,6 +146,7 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) { return (
+

{tt('공유 데이터소스입니다. AWS 계정 선택은 이 연결의 데이터 범위를 변경하지 않습니다.')}

{/* Always show the picker (preselected to the scoped instance) — never a dead-end if the scoped id isn't in the list yet. */} {( @@ -157,7 +162,7 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) { setEndpoint(e.target.value)} placeholder={ENDPOINT_PH[kind] ?? 'http://prometheus.internal:9090'} /> -

{tt('사설 주소는 운영자가 네트워크 연결과 DNS를 구성해야 사용할 수 있습니다.')}

@@ -185,9 +178,8 @@ export default function DatasourceForm({ result-cache TTL is deliberately not ported: the v2 query path is uncached by design) */}
- - setTimeoutS(e.target.value)} placeholder={tt(executionTimeout ? '기본 10' : '기본 12')} inputMode="numeric" /> -

{tt('적용 제한')}: {effectiveTimeout}s{!executionTimeout && ` · ${tt('서버 쿼리 중단을 보장하지 않습니다.')}`}

+ + setTimeoutS(e.target.value)} placeholder={tt('기본 10')} inputMode="numeric" /> {timeoutInvalid &&

{tt('1–60 사이의 정수를 입력하세요.')}

}
{kind === 'clickhouse' && ( @@ -197,18 +189,10 @@ export default function DatasourceForm({ {databaseInvalid &&

{tt('영문/숫자/밑줄 식별자만 가능하며 system 계열은 사용할 수 없습니다.')}

}
)} - {kind === 'mimir' && ( -
- - setApiPrefix(e.target.value)} placeholder="/prometheus" /> -

{tt('기본 /prometheus · API가 루트에 있으면 비워 두세요.')}

- {prefixInvalid &&

{tt('슬래시와 영문/숫자/밑줄/하이픈 경로만 사용할 수 있습니다.')}

} -
- )}
- {test && ( @@ -221,7 +205,7 @@ export default function DatasourceForm({ {err &&

{err}

}
- +
diff --git a/web/app/integrations/datasources/DatasourcesTab.test.tsx b/web/app/integrations/datasources/DatasourcesTab.test.tsx index dde63d31b..e7e4edeaf 100644 --- a/web/app/integrations/datasources/DatasourcesTab.test.tsx +++ b/web/app/integrations/datasources/DatasourcesTab.test.tsx @@ -17,17 +17,6 @@ beforeEach(() => { afterEach(() => { cleanup(); vi.restoreAllMocks(); }); describe('DatasourcesTab', () => { - it('uses explicit configuration evidence instead of a legacy connected flag', async () => { - global.fetch = vi.fn(async () => ({ - ok: true, status: 200, json: async () => ({ datasources: [{ - id: 1, name: 'unconfirmed-default', kind: 'prometheus', isDefault: true, - connected: true, configured: false, - }] }), - })) as unknown as typeof fetch; - render(); - expect(await screen.findByText('설정 미확인')).toBeTruthy(); - expect(screen.queryByText('● 연결됨')).toBeNull(); - }); it('lists instances (name/type/auth/default) with an Explore link per row', async () => { render(); await waitFor(() => expect(screen.getByText('prod-prom')).toBeTruthy()); @@ -59,8 +48,7 @@ describe('DatasourcesTab', () => { render(); await waitFor(() => expect(screen.getByText('prod-prom')).toBeTruthy()); expect(screen.getByText('총 데이터소스')).toBeTruthy(); - expect(screen.getAllByText('설정됨').length).toBeGreaterThan(0); - expect(screen.queryByText('● 연결됨')).toBeNull(); + expect(screen.getByText('연결됨')).toBeTruthy(); expect(screen.getByText('타입 종류')).toBeTruthy(); expect(screen.getByText('기본 데이터소스')).toBeTruthy(); // single default in this scenario → the tile names it diff --git a/web/app/integrations/datasources/DatasourcesTab.tsx b/web/app/integrations/datasources/DatasourcesTab.tsx index 085912e8d..ec36bcada 100644 --- a/web/app/integrations/datasources/DatasourcesTab.tsx +++ b/web/app/integrations/datasources/DatasourcesTab.tsx @@ -10,7 +10,7 @@ import DatasourceForm, { type DatasourceFormValue } from './DatasourceForm'; import { useI18n } from '@/components/shell/LanguageProvider'; interface Instance { - id: number; name: string; kind: string; endpoint?: string | null; authType?: string | null; isDefault?: boolean; connected?: boolean; configured?: boolean; settings?: { timeoutS?: number; database?: string; apiPrefix?: string }; + id: number; name: string; kind: string; endpoint?: string | null; authType?: string | null; isDefault?: boolean; connected?: boolean; settings?: { timeoutS?: number; database?: string }; } // Chat section per datasource kind (the deep-link prompt pins it with a leading /section — @@ -65,8 +65,7 @@ export default function DatasourcesTab({ canManage = false }: { canManage?: bool } // gap-audit L201: KPI roll-up from the already-fetched list — no extra API call. - const configured = (i: Instance) => i.configured ?? i.connected ?? false; - const configuredN = list.filter(configured).length; + const connectedN = list.filter((i) => i.connected).length; const kindN = new Set(list.map((i) => i.kind)).size; // Defaults are PER KIND — one arbitrary name would misrepresent the set, so show the count // (with every kind: name pair in a tooltip) once there's more than one. @@ -77,8 +76,7 @@ export default function DatasourcesTab({ canManage = false }: { canManage?: bool return (
-

{tt('관측성 데이터소스 (Prometheus·Mimir·Loki·Tempo·ClickHouse). 같은 타입 여러 개 등록 가능.')} - {' '}{tt('설정 상태는 실제 연결 성공을 의미하지 않습니다.')}

+

{tt('관측성 데이터소스 (Prometheus·Mimir·Loki·Tempo·ClickHouse). 같은 타입 여러 개 등록 가능.')}

{/* gap-audit L202: re-fetch without a full page reload. */} }
-

{tt('기본 데이터소스는 타입별 공유 연결이며 AWS 계정 선택으로 바뀌지 않습니다.')}

- +
`${d.kind}: ${d.name}`).join(', ')}> @@ -131,7 +128,7 @@ export default function DatasourcesTab({ canManage = false }: { canManage?: bool )} {i.authType ?? 'none'} - {configured(i) ? tt('설정됨') : tt('설정 미확인')} + {i.connected ? tt('● 연결됨') : tt('○ 미설정')} {i.isDefault ? {tt('★ 기본')} : (canManage && )} diff --git a/web/components/datasources/ExplorePanel.tsx b/web/components/datasources/ExplorePanel.tsx index 4abdc95c5..f112d8eab 100644 --- a/web/components/datasources/ExplorePanel.tsx +++ b/web/components/datasources/ExplorePanel.tsx @@ -15,7 +15,6 @@ import HBarList from '@/components/charts/HBarList'; import type { NormalizedResult } from '@/lib/datasource-render'; import { cn } from '@/lib/cn'; import { useI18n } from '@/components/shell/LanguageProvider'; -import { TIME_WINDOW_KINDS, TOOL } from '@/lib/datasource-query-tools'; // A datasource INSTANCE (the hub model): identified by bigint id, with a user-given name. export interface DatasourceInstance { @@ -37,7 +36,7 @@ const PH: Record = { dynatrace: 'metricSelector… 예: builtin:host.cpu.usage:avg', datadog: '메트릭 쿼리… 예: avg:system.cpu.user{*}', }; -const RANGE_KINDS = new Set(TIME_WINDOW_KINDS); +const RANGE_KINDS = new Set(['prometheus', 'mimir', 'loki']); // kinds with a *_query_range tool const selectCls = 'rounded-md border border-ink-200 bg-card px-2.5 py-1.5 text-[13px] text-ink-700'; // Explore range presets: label → window seconds (0 = instant snapshot). Step auto-derived for ~250 points. @@ -77,10 +76,7 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) { const ds = list.find((d) => d.id === selId) ?? null; const canRange = ds ? RANGE_KINDS.has(ds.kind) : false; // Loki has no per-request upstream timeout — the API caps it at 7d, so don't offer 30d. - const searchWindow = ds ? TOOL[ds.kind]?.timeWindow === true : false; - const effectiveRangeWindow = searchWindow ? rangeWindow || 3600 : rangeWindow; - const presets = RANGE_PRESETS.filter(([, seconds]) => - (!searchWindow || seconds > 0) && (!ds || !['loki', 'tempo', 'jaeger'].includes(ds.kind) || seconds <= 604800)); + const presets = ds?.kind === 'loki' ? RANGE_PRESETS.filter(([, s]) => s <= 604800) : RANGE_PRESETS; const queryIsMultiline = ds?.kind === 'clickhouse'; useEffect(() => { @@ -102,7 +98,7 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) { const run = useCallback(async (windowOverride?: number, queryOverride?: string) => { const q = queryOverride ?? query; // a quick-query chip can run its expr without waiting on setQuery if (selId === '' || !q.trim()) return; - const w = windowOverride ?? effectiveRangeWindow; + const w = windowOverride ?? rangeWindow; const kindNow = list.find((d) => d.id === selId)?.kind; // bind kind to THIS query, not the live selection const range = canRange && w > 0 ? { window: w, step: autoStep(w, kindNow) } : false; const queriedKind = kindNow; @@ -120,7 +116,7 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) { } catch (e) { setErr(e instanceof Error ? e.message : tt('쿼리 실패')); } finally { setBusy(false); } - }, [selId, query, effectiveRangeWindow, canRange, list, tt]); + }, [selId, query, rangeWindow, canRange, list, tt]); // NL → query (AI drafts, user reviews, then runs). Never auto-runs. const generate = useCallback(async () => { @@ -146,7 +142,6 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) { return (
-

{tt('공유 데이터소스입니다. AWS 계정 선택은 이 연결의 데이터 범위를 변경하지 않습니다.')}

{/* Always show the picker (preselected to the scoped instance) — never a dead-end if the scoped id isn't in the list yet. */} {( @@ -162,7 +157,7 @@ export default function ExplorePanel({ instanceId }: { instanceId?: number }) {