diff --git a/AGENTS.md b/AGENTS.md index a733625d..9f13e415 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -24,7 +24,7 @@ mcp-server/ │ ├── telemetry.py # Fire-and-forget community telemetry reporting │ ├── client_meta.py # Shared client metadata extraction helpers and defaults │ ├── observability.py # Resource-read observability helpers -│ ├── pro_api_key_context.py # Request-scoped client-supplied PRO API key state, resolver, and @pro_api_key_scope decorator; per-invocation credit sink and @pro_api_credit_scope decorator +│ ├── pro_api_key_context.py # Request-scoped client-supplied PRO API key state, resolver, and @pro_api_key_scope decorator; per-invocation credit sink and @pro_api_credit_scope decorator; auth-origin and key-fingerprint helpers for analytics/telemetry │ ├── cache.py # Simple in-memory cache for chain data │ ├── web3_pool.py # Async Web3 connection pool manager │ ├── models.py # Defines standardized Pydantic models for all tool responses @@ -115,6 +115,7 @@ mcp-server/ │ │ ├── test_routes.py # Unit tests for API route definitions │ │ └── test_skill_resource_routes.py # Unit tests for the bundled skill HTTP mirror │ ├── conftest.py +│ ├── pro_api_key_helpers.py # Shared request-context builders for PRO API key / auth-signal tests │ ├── evals/ # Evaluation artifacts and runner configs for tool output checks │ │ ├── .env.example │ │ ├── .gemini/ @@ -135,6 +136,7 @@ mcp-server/ │ ├── test_client_meta.py # Unit tests for client metadata extraction │ ├── test_observability.py # Unit tests for resource-read observability │ ├── test_pro_api_key_context.py # Unit tests for client-supplied PRO API key resolution +│ ├── test_pro_api_key_context_auth_signals.py # Unit tests for ctx-derived auth-origin and PRO API key fingerprint signals │ ├── test_hatch_build.py # Unit tests for custom Hatch build hook helpers │ ├── test_instructions_data.py # Unit tests for the InstructionsData payload model │ ├── test_integration_helpers.py # Unit tests for integration test helpers @@ -145,6 +147,7 @@ mcp-server/ │ ├── test_bundled_skill_artifacts.py # Unit tests for bundled skill packaging artifacts │ ├── test_skill_resources_server.py # Unit tests for MCP resource registration │ ├── test_telemetry.py # Unit tests for telemetry reporting +│ ├── test_tool_usage_report.py # Unit tests for the ToolUsageReport telemetry payload model │ ├── test_web3_pool.py # Unit tests for web3 pool management │ ├── resources/ # Unit tests for server-owned resource modules │ │ ├── __init__.py @@ -398,6 +401,7 @@ mcp-server/ * **`pro_api_key_context.py`**: * Owns request-scoped resolution of a client-supplied Blockscout PRO API key, kept separate from logging/observability. * Provides a `ContextVar` of the per-request client-key state, a normalization/validation helper, `extract_client_pro_api_key_from_ctx()`, `resolve_pro_api_key()` (precedence: valid client key → server key → not-configured error; malformed client key → terminal error, no fallback), and the `@pro_api_key_scope` decorator. + * Also provides the `ctx`-derived helper `compute_auth_signals()`, used by the analytics and community-telemetry paths. * Honored for any HTTP request that carries the configured header (MCP-over-HTTP or REST); the key is never logged or placed in cache keys. * Also defines the per-invocation credit-tracking symbols: `CreditSink`, the `_credit_sink` `ContextVar`, and the `@pro_api_credit_scope` decorator (a sibling of `@pro_api_key_scope`). * **`cache.py`**: diff --git a/API.md b/API.md index 48158deb..ab026451 100644 --- a/API.md +++ b/API.md @@ -671,6 +671,8 @@ Receive an anonymous tool usage report from a community-run server. | `client_name` | `string` | Yes | Name of the MCP client invoking the tool. | | `client_version` | `string` | Yes | Version of the MCP client. | | `protocol_version` | `string` | Yes | Model Context Protocol version used. | + | `auth_origin` | `string \| null` | No | Resolved authorization origin on the reporting instance: `client`, `server`, or `none`. Explicit `null` (or omitted, as from legacy reporters) is treated as `unknown` downstream. | + | `api_key_fingerprint` | `string \| null` | No | One-way, non-reversible SHA-256 fingerprint of the effective PRO API key available to back the request on the reporting instance — the request's authorization context, not whether the invoked tool consumed the key. Non-null values are exactly 64 lowercase hexadecimal characters; never the key itself; explicit `null` (or omitted) when no usable key was available. | - **Example Request** diff --git a/README.md b/README.md index 03f1365b..82a4950b 100644 --- a/README.md +++ b/README.md @@ -454,10 +454,11 @@ To help us improve the Blockscout MCP Server, community-run instances of the ser - The name of the tool being called (e.g., `get_block_number`). - The parameters provided to the tool. - The version of the Blockscout MCP Server being used. +- A one-way, non-reversible hash (SHA-256) of the PRO API key available to authorize the request, when one is present. This is a derived fingerprint only — the key itself is never transmitted and cannot be recovered from the hash. **What we DO NOT collect:** -- We do not collect any personal data, IP addresses (the central server uses the sender's IP for geolocation via Mixpanel and then discards it), secrets, or private keys. +- We do not collect any personal data, IP addresses (the central server uses the sender's IP for geolocation via Mixpanel and then discards it), or secrets and private keys themselves. The PRO API key in particular is never transmitted — only its one-way, non-reversible fingerprint described above, from which the key cannot be recovered. ### How to Opt-Out diff --git a/SPEC.md b/SPEC.md index b16aca0a..0efad576 100644 --- a/SPEC.md +++ b/SPEC.md @@ -897,6 +897,7 @@ To gain insight into tool usage patterns, the server can optionally report tool - MCP protocol version. - Tool arguments (currently sent as-is, without truncation). - Call source: whether the tool was invoked by MCP or via the REST API. + - Authorization origin: whether the request was backed by a client-supplied PRO API key (`client`), the server-configured key (`server`), or no usable key (`none`). This dimension is orthogonal to the call source — the two compose to describe both how a call arrived and how it was authorized. Community-forwarded events that predate this signal are recorded as `unknown`. ##### Intent Inference from Tool Sequences @@ -923,6 +924,7 @@ Intent analytics should favor derived signals over raw text. - A stable `distinct_id` is generated to anonymously identify unique users. - The fingerprint is the concatenation of: namespace URL (`https://mcp.blockscout.com/mcp`), client IP, client name, and client version. - This yields stable identification even when multiple clients share the same name/version (e.g., Claude Desktop) because their IPs differ. + - A future iteration will fold the effective-key fingerprint into `distinct_id` on this direct path: when the caller supplies a client key, the client-key fingerprint identifies the user; absent a client key, the composite stays IP + client name + version — the public server's server key is shared by all callers, so it is deliberately **not** used as an identity. This deferred follow-up (see *Community Telemetry* below) is why the effective-key fingerprint is derived whenever any telemetry sink is active, not only when community telemetry is enabled, even though only community reports consume it today. - REST API support and source attribution: - The REST context mock is extended with a request context wrapper so analytics can extract IP and headers consistently (see `blockscout_mcp_server/api/dependencies.py`). @@ -935,6 +937,7 @@ Intent analytics should favor derived signals over raw text. - **HTTP mode**: Active only when both `BLOCKSCOUT_MIXPANEL_TOKEN` is not configured AND `BLOCKSCOUT_DISABLE_COMMUNITY_TELEMETRY` is not set to true - **Mechanism**: To understand usage in the open-source community, these instances send an anonymous, "fire-and-forget" report to a central endpoint (`POST /v1/report_tool_usage`) on the official Blockscout MCP server. This report contains the tool name, tool arguments, the MCP client name and version, the model context protocol version, and the server's version. - **Central Processing**: The central server receives this report, uses the sender's IP address for geolocation, and forwards the event to Mixpanel with the client metadata, protocol version, and a `source` property of `"community"`. This allows us to gather valuable aggregate statistics without requiring every user to have a Mixpanel account. +- **Authorization context**: Community reports also carry the request's authorization origin and a one-way, non-reversible fingerprint of the effective PRO API key, so direct and community analytics share the same authorization-context dimension. The raw key never leaves the instance — only the fingerprint, and only when a usable key was available. Because these reports arrive fire-and-forget from independently-versioned community instances, both new fields tolerate unrecognized wire values rather than dropping an otherwise-valid report. Consuming and retaining the fingerprint for stronger unique-user identity — the client-key fingerprint per user, or on a personal self-hosted instance the server-key fingerprint per instance — together with a server-side HMAC pepper, is deferred to a dedicated follow-up. (Exact field names, enum values, and hex-shape constraints are documented in `API.md`; the coercion behavior lives in the `ToolUsageReport` validators.) - **Opt-Out**: This community reporting can be completely disabled by setting the `BLOCKSCOUT_DISABLE_COMMUNITY_TELEMETRY` environment variable to `true`. ##### Resource-Read Observability diff --git a/blockscout_mcp_server/__init__.py b/blockscout_mcp_server/__init__.py index 0a765d1c..6e671f85 100644 --- a/blockscout_mcp_server/__init__.py +++ b/blockscout_mcp_server/__init__.py @@ -1,4 +1,4 @@ # SPDX-License-Identifier: LicenseRef-Blockscout """Blockscout MCP Server package.""" -__version__ = "0.17.0.dev1" +__version__ = "0.17.0.dev2" diff --git a/blockscout_mcp_server/analytics.py b/blockscout_mcp_server/analytics.py index af0ba89f..d52359f0 100644 --- a/blockscout_mcp_server/analytics.py +++ b/blockscout_mcp_server/analytics.py @@ -37,7 +37,7 @@ def __init__(self, *args: Any, **kwargs: Any) -> None: # noqa: D401 - simple pl get_header_case_insensitive, ) from blockscout_mcp_server.config import config -from blockscout_mcp_server.constants import RESOURCE_READ_EVENT +from blockscout_mcp_server.constants import AUTH_ORIGIN_UNKNOWN, RESOURCE_READ_EVENT, AuthOrigin from blockscout_mcp_server.models import ToolUsageReport logger = logging.getLogger(__name__) @@ -191,8 +191,20 @@ def track_tool_invocation( tool_name: str, tool_args: dict[str, Any], client_meta: ClientMeta | None = None, + auth_origin: AuthOrigin | None = None, ) -> None: - """Track a tool invocation in Mixpanel, if enabled and in HTTP mode.""" + """Track a tool invocation in Mixpanel, if enabled and in HTTP mode. + + ``auth_origin`` is the pre-computed origin threaded in by the caller — the + observability paths derive the auth signals once per invocation (via + :func:`blockscout_mcp_server.telemetry.resolve_auth_signals`) and reuse them + for both this sink and the community report, so the origin is never + re-derived here. A ``None`` value — which ``resolve_auth_signals`` yields + when derivation is skipped or degrades — is recorded as + ``AUTH_ORIGIN_UNKNOWN``, mirroring :func:`track_community_usage`. Re-deriving + from ``ctx`` at this point would re-run the very computation that just failed + and lose the whole event, so it is deliberately avoided. + """ if not _is_http_mode_enabled: return mp = _get_mixpanel_client() @@ -225,6 +237,7 @@ def track_tool_invocation( "tool_args": tool_args, "protocol_version": protocol_version, "source": _determine_call_source(ctx), + "auth_origin": auth_origin if auth_origin is not None else AUTH_ORIGIN_UNKNOWN, } meta = {"ip": ip} if ip else None @@ -241,15 +254,18 @@ def track_resource_read( ctx: Any, uri: str, client_meta: ClientMeta | None = None, + auth_origin: AuthOrigin | None = None, ) -> None: """Track a resource read in Mixpanel, if enabled and in HTTP mode. Delegates to :func:`track_tool_invocation` using the ``RESOURCE_READ`` event sentinel so that all gating logic (HTTP-mode, token, IP extraction, etc.) is reused verbatim. The caller is responsible for providing a fully-normalised - URI string — this function does not stringify. + URI string — this function does not stringify. ``auth_origin`` is threaded + through to :func:`track_tool_invocation` (see its docstring) so the resource + observability path also derives the auth signals only once per read. """ - track_tool_invocation(ctx, RESOURCE_READ_EVENT, {"uri": uri}, client_meta=client_meta) + track_tool_invocation(ctx, RESOURCE_READ_EVENT, {"uri": uri}, client_meta=client_meta, auth_origin=auth_origin) def track_community_usage(report: ToolUsageReport, ip: str, user_agent: str) -> None: @@ -271,6 +287,7 @@ def track_community_usage(report: ToolUsageReport, ip: str, user_agent: str) -> "tool_args": report.tool_args, "protocol_version": report.protocol_version, "source": "community", + "auth_origin": report.auth_origin if report.auth_origin is not None else AUTH_ORIGIN_UNKNOWN, } meta = {"ip": ip} if ip else None diff --git a/blockscout_mcp_server/constants.py b/blockscout_mcp_server/constants.py index a4aca0ab..a50cac25 100644 --- a/blockscout_mcp_server/constants.py +++ b/blockscout_mcp_server/constants.py @@ -1,6 +1,8 @@ # SPDX-License-Identifier: LicenseRef-Blockscout """Constants used throughout the Blockscout MCP Server.""" +from typing import Literal + from blockscout_mcp_server import __version__ SERVER_VERSION = __version__ @@ -104,3 +106,21 @@ # The maximum length for a transaction's input data field before it's truncated. # 514 = '0x' prefix + 512 hex characters (256 bytes). INPUT_DATA_TRUNCATION_LIMIT = 514 + +# Versioned domain-separation prefix for the PRO API key fingerprint hash. The "v1" is +# deliberate: it lets the hashing scheme be versioned later without silently colliding +# with old fingerprints. It is not a secret and provides domain separation, not +# brute-force resistance (PRO API keys are high-entropy, so preimage attacks are +# infeasible; a server-side HMAC pepper is deferred to a follow-up change). +PRO_API_KEY_HASH_PREFIX = "bs-pro-key-v1:" + +# The three real authorization origins a key can come from, as computed by the +# ctx-derived helpers. Defined as a single Literal alias (not separate string +# constants) because a Literal[...] annotation cannot be built from string-constant +# variables; the model field and the helper return type both import this alias. +AuthOrigin = Literal["client", "server", "none"] + +# Legacy sentinel used only at the Mixpanel layer as the default for community +# reports that predate the `auth_origin` field. Deliberately not part of `AuthOrigin`: +# it is never a valid report value and never returned by the helpers. +AUTH_ORIGIN_UNKNOWN = "unknown" diff --git a/blockscout_mcp_server/models.py b/blockscout_mcp_server/models.py index 8cef6239..a5a26730 100644 --- a/blockscout_mcp_server/models.py +++ b/blockscout_mcp_server/models.py @@ -1,9 +1,24 @@ # SPDX-License-Identifier: LicenseRef-Blockscout """Pydantic models for standardized tool responses.""" -from typing import Any, Generic, TypeVar +import logging +import re +from typing import Any, Generic, TypeVar, get_args -from pydantic import BaseModel, ConfigDict, Field +from pydantic import BaseModel, ConfigDict, Field, field_validator + +from blockscout_mcp_server.constants import AuthOrigin + +logger = logging.getLogger(__name__) + +# A fingerprint is exactly 64 lowercase hex characters — the shape of a +# hashlib.sha256(...).hexdigest() digest. +_FINGERPRINT_PATTERN = re.compile(r"[0-9a-f]{64}") + +# The wire-valid `auth_origin` values, derived from the `AuthOrigin` Literal so the two +# never drift. A value outside this set (plus JSON null) is coerced to None rather than +# rejected — see ``_tolerate_unknown_auth_origin``. +_VALID_AUTH_ORIGINS = frozenset(get_args(AuthOrigin)) # --- Generic Type Variable --- T = TypeVar("T") @@ -15,6 +30,71 @@ class ToolUsageReport(BaseModel): client_name: str client_version: str protocol_version: str + auth_origin: AuthOrigin | None = Field( + default=None, + description=( + "The origin of the authorization available to back the reported call: 'client' for a " + "client-supplied PRO API key, 'server' for a server-configured key, or 'none' for " + "no usable key. Reflects the request's authorization context, not whether the invoked " + "tool actually consumed a key. Absent on legacy payloads that predate this field. An " + "unrecognized value (unknown enum member, wrong case, or non-string) is coerced to null " + "and reported as 'unknown' downstream, so a version-skewed reporter never drops an " + "otherwise-valid report." + ), + ) + api_key_fingerprint: str | None = Field( + default=None, + description=( + "A one-way, non-reversible SHA-256 hex digest fingerprint of the effective PRO API " + "key available to back the reported call, or null if no usable key was available. " + "A valid value is exactly 64 lowercase hex characters. Because this field is accepted " + "over the wire but not yet consumed (not forwarded to Mixpanel, not persisted), a " + "malformed value is tolerated: it is coerced to null rather than rejecting the " + "otherwise-valid report." + ), + ) + + @field_validator("auth_origin", mode="before") + @classmethod + def _tolerate_unknown_auth_origin(cls, value: Any) -> str | None: + """Coerce an unrecognized ``auth_origin`` to ``None`` (→ ``unknown`` downstream) instead of rejecting. + + ``auth_origin`` is consumed as a Mixpanel dimension, but the reports arrive fire-and-forget + from independently-versioned community instances, so a value this receiver does not (yet) + recognize — a future enum member rolled out to reporters first, wrong capitalization, or + non-string junk — must not 422 the whole otherwise-valid report. Anything outside + ``_VALID_AUTH_ORIGINS`` (plus JSON null) is coerced to ``None``, which + :func:`analytics.track_community_usage` already renders as the ``unknown`` bucket that legacy + (field-absent) reports land in. This keeps both forward-compat wire signals symmetric with + ``api_key_fingerprint`` below. The debug log preserves visibility into version skew that a + silent 422 (dropped on both ends) would hide; it records only the value's type and length — + never the value itself — mirroring :meth:`_tolerate_malformed_fingerprint`. + """ + if value is None or (isinstance(value, str) and value in _VALID_AUTH_ORIGINS): + return value + logger.debug( + "Coercing unrecognized auth_origin to None (type=%s, len=%s)", + type(value).__name__, + len(value) if isinstance(value, str) else "n/a", + ) + return None + + @field_validator("api_key_fingerprint", mode="before") + @classmethod + def _tolerate_malformed_fingerprint(cls, value: Any) -> str | None: + """Coerce a malformed, not-yet-consumed fingerprint to ``None`` instead of rejecting. + + The fingerprint is a forward-compatible wire signal that no consumer reads yet, so one + malformed value must not drop an otherwise-valid community report. Any value that is not + ``None`` and not a ``str`` matching the 64-lowercase-hex ``_FINGERPRINT_PATTERN`` (including + non-string junk) is coerced to ``None``, preserving the "present ⇒ valid 64-hex" invariant + for the deferred identity follow-up. ``auth_origin`` is coerced the same way (to ``None`` → + ``unknown``) by :meth:`_tolerate_unknown_auth_origin` above. + """ + if value is None or (isinstance(value, str) and _FINGERPRINT_PATTERN.fullmatch(value)): + return value + logger.debug("Coercing malformed api_key_fingerprint to None (type=%s)", type(value).__name__) + return None # --- Models for Pagination --- diff --git a/blockscout_mcp_server/observability.py b/blockscout_mcp_server/observability.py index aab8c931..aebce420 100644 --- a/blockscout_mcp_server/observability.py +++ b/blockscout_mcp_server/observability.py @@ -69,9 +69,14 @@ def log_resource_read(uri: Any, ctx: Any) -> None: except Exception: pass + # Derive the auth-origin / fingerprint signals once and reuse them for both + # sinks below (mirrors @log_tool_invocation); see telemetry.resolve_auth_signals + # for the rationale and gating. + auth_origin, api_key_fingerprint = telemetry.resolve_auth_signals(ctx) + # Step 2 — direct analytics sink (self-gating, synchronous). try: - analytics.track_resource_read(ctx, full_uri, client_meta=meta) + analytics.track_resource_read(ctx, full_uri, client_meta=meta, auth_origin=auth_origin) except Exception: pass @@ -95,6 +100,8 @@ def log_resource_read(uri: Any, ctx: Any) -> None: meta.name, meta.version, meta.protocol, + auth_origin=auth_origin, + api_key_fingerprint=api_key_fingerprint, ) ) except Exception: diff --git a/blockscout_mcp_server/pro_api_key_context.py b/blockscout_mcp_server/pro_api_key_context.py index fc60ad6b..e67278a4 100644 --- a/blockscout_mcp_server/pro_api_key_context.py +++ b/blockscout_mcp_server/pro_api_key_context.py @@ -9,6 +9,9 @@ - A normalization/validation helper. - An extractor that reads the key from any HTTP request context (MCP-over-HTTP or REST). +- A single precedence decision (``_apply_key_precedence``) shared by the resolver + and the analytics signal derivation, so the enforced key and the reported + ``auth_origin`` cannot drift apart. - A resolver that applies the client-key → server-key precedence rule. - A ``require_pro_api_key()`` helper that wraps the resolver with the standard not-configured error so every PRO API entry point raises the same message. @@ -48,6 +51,7 @@ from __future__ import annotations import functools +import hashlib import inspect import logging import math @@ -58,6 +62,7 @@ from blockscout_mcp_server.client_meta import get_header_case_insensitive from blockscout_mcp_server.config import config +from blockscout_mcp_server.constants import PRO_API_KEY_HASH_PREFIX, AuthOrigin logger = logging.getLogger(__name__) @@ -99,6 +104,69 @@ class _Malformed: _MALFORMED: ClientKeyState = _Malformed() +# --------------------------------------------------------------------------- +# 1b. Precedence decision — the single source of truth over ClientKeyState +# --------------------------------------------------------------------------- +# +# The client-key → server-key precedence is decided in exactly one place, +# ``_apply_key_precedence``. Both the enforcement path +# (``resolve_pro_api_key``) and the telemetry path (``compute_auth_signals``) +# consume its result and only *format* it — the former with raise/ContextVar +# semantics, the latter with an ``(origin, fingerprint)`` pair. Because neither +# re-decides precedence, the enforced key and the reported ``auth_origin`` can +# never disagree, and a future precedence change (a new key source, different +# handling of a malformed key) is a one-line edit here that both paths follow +# automatically. This is the consistency issue #423 exists to protect. + + +@dataclass(frozen=True) +class _UseClientKey: + """A valid client key won precedence; ``key`` is the effective PRO API key.""" + + key: str + + +@dataclass(frozen=True) +class _RejectMalformed: + """A malformed client key was supplied — reject it with no fallback to the server key.""" + + +@dataclass(frozen=True) +class _UseServerKey: + """No client key; the configured server key is effective (``key`` is ``""`` when unset).""" + + key: str + + +# The three mutually exclusive precedence outcomes. +_AuthDecision = _UseClientKey | _RejectMalformed | _UseServerKey + + +def _apply_key_precedence(state: ClientKeyState) -> _AuthDecision: + """Map a client-key *state* to the effective auth decision (single source of truth). + + Precedence — this **is** the definition that both :func:`resolve_pro_api_key` + and :func:`compute_auth_signals` consume; it is not mirrored anywhere else: + + 1. Valid client key → :class:`_UseClientKey` (use the client value). + 2. Malformed client key → :class:`_RejectMalformed` (no fallback to the server + key — the request must fail rather than silently downgrade to the server). + 3. No client key (absent) → :class:`_UseServerKey` carrying + ``config.pro_api_key`` (which may be ``""`` when no server key is + configured). + + Pure and total over :data:`ClientKeyState`: it never raises and never hashes. + The raise/ContextVar semantics live in :func:`resolve_pro_api_key`; the + origin/hashing semantics live in :func:`compute_auth_signals`. + """ + if isinstance(state, _Valid): + return _UseClientKey(key=state.value) + if isinstance(state, _Malformed): + return _RejectMalformed() + # _Absent — fall back to the configured server key (may be ""). + return _UseServerKey(key=config.pro_api_key) + + # --------------------------------------------------------------------------- # 2. Module-level ContextVars (not per-call — required for correct semantics) # --------------------------------------------------------------------------- @@ -231,6 +299,95 @@ def extract_client_pro_api_key_from_ctx(ctx: Any) -> ClientKeyState: return _ABSENT +# --------------------------------------------------------------------------- +# 4b. ctx-derived auth-origin and fingerprint helpers (for analytics/telemetry) +# --------------------------------------------------------------------------- +# +# Analytics and telemetry (e.g. log_tool_invocation / community reporting) run +# *outside* @pro_api_key_scope, so they cannot read the _client_key_state +# ContextVar (see the @pro_api_key_scope docstring above). These helpers read the +# key state directly from ctx headers via extract_client_pro_api_key_from_ctx() +# and then apply the *same* precedence through _apply_key_precedence() — only the +# state *source* differs (ctx headers here vs the ContextVar on the enforcement +# path). They must NOT call resolve_pro_api_key(): it reads the request-scoped +# ContextVar (unset on this path) and raises on a malformed key, whereas these +# helpers must never raise. + + +def _fingerprint_pro_api_key(key: str) -> str: + """Return the hex SHA-256 digest of *key*, domain-separated by ``PRO_API_KEY_HASH_PREFIX``. + + ``hashlib.sha256`` takes a bytes-like object, not a ``str``; the + prefix+encoding scheme is centralized here so both the client-key and + server-key branches of :func:`compute_auth_signals` share one construction. + The raw key is never returned, logged, or placed anywhere but this hash + input. + """ + return hashlib.sha256(f"{PRO_API_KEY_HASH_PREFIX}{key}".encode()).hexdigest() + + +@functools.lru_cache(maxsize=1) +def _server_api_key_fingerprint(key: str) -> str: + """Return the fingerprint of the configured server key, memoized per process. + + ``config.pro_api_key`` is fixed for the process lifetime, so its SHA-256 is a + hash of a constant. Memoizing it means the observability hot path (every tool + call / resource read) reuses the one digest instead of re-hashing the same + value each time. ``maxsize=1`` keeps a single entry: production never varies + the key, and tests that monkeypatch ``config.pro_api_key`` cache-miss on the + new value and evict the stale one. + + Only the *server* key is memoized. Client keys are per-request secrets, so they + stay on the un-cached :func:`_fingerprint_pro_api_key` path and are never + retained in this cache. + """ + return _fingerprint_pro_api_key(key) + + +def compute_auth_signals(ctx: Any) -> tuple[AuthOrigin, str | None]: + """Derive the ``(auth_origin, api_key_fingerprint)`` pair for *ctx* in one pass. + + Single source of truth for both signals: each precedence branch returns the + origin and the matching fingerprint together, so the two can never disagree + (``"client"`` ↔ client hash, ``"server"`` ↔ server hash, ``"none"`` ↔ + ``None``). Callers that need only the origin can discard the fingerprint + (``[1]``). + + Never raises (delegates to :func:`extract_client_pro_api_key_from_ctx`, + which never raises) and never calls :func:`resolve_pro_api_key` (it reads the + request-scoped ContextVar — unset on this path — and raises on a malformed + key). Precedence is not re-decided here: it delegates to + :func:`_apply_key_precedence` — the very decision :func:`resolve_pro_api_key` + enforces — and only maps the outcome to an origin + fingerprint: + + - :class:`_UseClientKey` → ``("client", )``. + - :class:`_RejectMalformed` → ``("none", None)`` (a malformed submission has + no usable key and will fail at the PRO API chokepoint anyway). + - :class:`_UseServerKey` → ``("server", )`` when a + server key is configured, otherwise ``("none", None)``. + + The server-key hash is memoized (see :func:`_server_api_key_fingerprint`) so + the observability hot path never re-hashes the fixed server key. The + fingerprint's sole consumer is the community usage report; the analytics sink + only reads the origin. + + The raw key is never returned, logged, or used anywhere but as the hash + input. + """ + decision = _apply_key_precedence(extract_client_pro_api_key_from_ctx(ctx)) + + if isinstance(decision, _UseClientKey): + return "client", _fingerprint_pro_api_key(decision.key) + + if isinstance(decision, _RejectMalformed): + return "none", None + + # _UseServerKey — "server" only when a key is actually configured. + if decision.key: + return "server", _server_api_key_fingerprint(decision.key) + return "none", None + + # --------------------------------------------------------------------------- # 5. Resolver — applies client-key → server-key precedence # --------------------------------------------------------------------------- @@ -239,7 +396,11 @@ def extract_client_pro_api_key_from_ctx(ctx: Any) -> ClientKeyState: def resolve_pro_api_key() -> str: """Return the effective PRO API key for the current request. - Precedence: + Precedence is decided by :func:`_apply_key_precedence` (shared with + :func:`compute_auth_signals`, so the enforced key and the reported + ``auth_origin`` can never drift apart). This function only applies the + ContextVar/raise semantics to that decision: + 1. Client-supplied key (valid) → use it. 2. Client-supplied key (malformed) → raise ``ValueError`` immediately. No fallback to the server key for a malformed submission. @@ -247,19 +408,19 @@ def resolve_pro_api_key() -> str: Callers' existing emptiness guards handle the ``""`` case. """ - state = _client_key_state.get() + decision = _apply_key_precedence(_client_key_state.get()) - if isinstance(state, _Valid): - return state.value + if isinstance(decision, _UseClientKey): + return decision.key - if isinstance(state, _Malformed): + if isinstance(decision, _RejectMalformed): raise ValueError( "The supplied PRO API key header value is malformed: it contains control characters " "or exceeds the maximum allowed length. Please provide a valid key." ) - # _Absent — fall back to the configured server key. - return config.pro_api_key + # _UseServerKey — the configured server key, possibly "". + return decision.key # --------------------------------------------------------------------------- diff --git a/blockscout_mcp_server/telemetry.py b/blockscout_mcp_server/telemetry.py index 7053eb20..d143234f 100644 --- a/blockscout_mcp_server/telemetry.py +++ b/blockscout_mcp_server/telemetry.py @@ -1,5 +1,6 @@ # SPDX-License-Identifier: LicenseRef-Blockscout import logging +from typing import Any import httpx @@ -10,19 +11,76 @@ COMMUNITY_TELEMETRY_URL, RESOURCE_READ_EVENT, SERVER_VERSION, + AuthOrigin, ) +from blockscout_mcp_server.pro_api_key_context import compute_auth_signals logger = logging.getLogger(__name__) +def is_any_telemetry_active() -> bool: + """Return whether any telemetry sink can still emit under the current config. + + Single source of truth for the "is it worth deriving the request's auth + identity at all?" precondition, replacing the inline boolean that used to + duplicate the analytics and community sinks' own gates at each derivation site. + Kept deliberately coarse and conservative: it is a *superset* of the union of + the two sinks' precise send conditions, so it reports *inactive* only when + telemetry is provably off (not HTTP mode **and** community telemetry disabled). + Erring toward "active" keeps callers independent of each sink's internal logic; + the sinks still self-gate before actually sending. + """ + return analytics.is_http_mode_enabled() or not config.disable_community_telemetry + + +def resolve_auth_signals(ctx: Any) -> tuple[AuthOrigin | None, str | None]: + """Derive the ``(auth_origin, api_key_fingerprint)`` pair for the observability sinks. + + Single entry point shared by both observability paths — ``log_tool_invocation`` + (the tool decorator) and ``log_resource_read`` — so the one ``ctx`` extraction + and SHA-256 (in :func:`compute_auth_signals`), the defensive guard, and the + all-telemetry-disabled short-circuit are written once instead of duplicated at + each site. Returns ``(None, None)`` without touching ``ctx`` when no sink can + consume the signals (see :func:`is_any_telemetry_active`); both sinks + short-circuit on their own gates in that state anyway, so nothing emitted + changes while the ``ctx`` extraction and key hashing are skipped. + + The fingerprint is forward-provisioned: today only the community usage report + consumes it, but it is intended to key Mixpanel ``distinct_id`` per + user/instance depending on deployment (see SPEC.md -> Performance Optimizations + -> Dual-Mode Analytics). That is why signals are derived whenever *any* + telemetry sink is active, not only when community telemetry is enabled. + + Never raises: :func:`compute_auth_signals` is defensive today, and the gate is + evaluated *inside* the ``try`` so this observability concern can never propagate + into the tool body even if either contract later changes. The ``(None, None)`` + fallback degrades gracefully — the analytics sink records the origin as + ``AUTH_ORIGIN_UNKNOWN`` (it never re-derives from ``ctx``), the community report + omits the hash. + """ + try: + if not is_any_telemetry_active(): + return None, None + return compute_auth_signals(ctx) + except Exception: + return None, None + + async def send_community_usage_report( tool_name: str, tool_args: dict, client_name: str, client_version: str, protocol_version: str, + auth_origin: str | None = None, + api_key_fingerprint: str | None = None, ) -> None: - """Send a fire-and-forget tool usage report if in community telemetry mode.""" + """Send a fire-and-forget tool usage report if in community telemetry mode. + + ``auth_origin`` and ``api_key_fingerprint`` are already-computed signals (see + :mod:`blockscout_mcp_server.pro_api_key_context`); this function is a dumb conduit + and must never receive or handle a raw API key. + """ if config.disable_community_telemetry: return @@ -37,6 +95,8 @@ async def send_community_usage_report( "client_name": client_name, "client_version": client_version, "protocol_version": protocol_version, + "auth_origin": auth_origin, + "api_key_fingerprint": api_key_fingerprint, } url = f"{COMMUNITY_TELEMETRY_URL}{COMMUNITY_TELEMETRY_ENDPOINT}" @@ -52,10 +112,20 @@ async def send_community_resource_report( client_name: str, client_version: str, protocol_version: str, + auth_origin: str | None = None, + api_key_fingerprint: str | None = None, ) -> None: """Send a fire-and-forget resource read report if in community telemetry mode. Delegates to :func:`send_community_usage_report` using the ``RESOURCE_READ`` event sentinel so all gating and POST logic is reused verbatim. """ - await send_community_usage_report(RESOURCE_READ_EVENT, {"uri": uri}, client_name, client_version, protocol_version) + await send_community_usage_report( + RESOURCE_READ_EVENT, + {"uri": uri}, + client_name, + client_version, + protocol_version, + auth_origin=auth_origin, + api_key_fingerprint=api_key_fingerprint, + ) diff --git a/blockscout_mcp_server/tools/decorators.py b/blockscout_mcp_server/tools/decorators.py index 9e262354..0cb6d897 100644 --- a/blockscout_mcp_server/tools/decorators.py +++ b/blockscout_mcp_server/tools/decorators.py @@ -29,6 +29,10 @@ async def wrapper(*args: Any, **kwargs: Any) -> Any: client_version = meta.version protocol_version = meta.protocol + # Derive the auth-origin / fingerprint signals once and reuse them for both + # sinks below; see telemetry.resolve_auth_signals for the rationale and gating. + auth_origin, api_key_fingerprint = telemetry.resolve_auth_signals(ctx) + # Track analytics (no-op if disabled) try: analytics.track_tool_invocation( @@ -36,6 +40,7 @@ async def wrapper(*args: Any, **kwargs: Any) -> Any: func.__name__, arg_dict, client_meta=meta, + auth_origin=auth_origin, ) except Exception: # Defensive: tracking must never break tool execution @@ -56,6 +61,8 @@ async def wrapper(*args: Any, **kwargs: Any) -> Any: client_name, client_version, protocol_version, + auth_origin=auth_origin, + api_key_fingerprint=api_key_fingerprint, ) ) except Exception: diff --git a/pyproject.toml b/pyproject.toml index bcdb50c7..ba93c7de 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "blockscout-mcp-server" -version = "0.17.0.dev1" +version = "0.17.0.dev2" description = "MCP server for Blockscout" requires-python = ">=3.11" dependencies = [ diff --git a/server.json b/server.json index d58dac80..a6f87b9f 100644 --- a/server.json +++ b/server.json @@ -2,7 +2,7 @@ "$schema": "https://static.modelcontextprotocol.io/schemas/2025-12-11/server.schema.json", "name": "com.blockscout/mcp-server", "description": "MCP server for Blockscout", - "version": "0.17.0.dev1", + "version": "0.17.0.dev2", "websiteUrl": "https://blockscout.com", "repository": { "url": "https://github.com/blockscout/mcp-server", diff --git a/tests/api/test_routes.py b/tests/api/test_routes.py index 0484bd99..1dfaac81 100644 --- a/tests/api/test_routes.py +++ b/tests/api/test_routes.py @@ -725,9 +725,23 @@ async def test_error_handling(mock_tool, client: AsyncClient, side_effect, statu mock_tool.assert_called_once_with(chain_id="1", ctx=ANY) -@pytest.mark.asyncio -@patch("blockscout_mcp_server.api.routes.analytics.track_community_usage") -async def test_report_tool_usage_success(mock_track, client: AsyncClient): +# --------------------------------------------------------------------------- +# /v1/report_tool_usage — shared payload/POST helper +# --------------------------------------------------------------------------- +# Every report-endpoint test posts the same base payload and User-Agent, varying +# only the auth-signal fields under test; ``post_report`` keeps each test to the +# fields it actually exercises. + +_REPORT_USER_AGENT = "BlockscoutMCP/0.0" + + +async def post_report(client: AsyncClient, **overrides): + """POST the base /v1/report_tool_usage payload (with *overrides* applied) and the standard User-Agent. + + Keyword *overrides* add or replace top-level payload fields, so a test can set + ``auth_origin``/``api_key_fingerprint`` (to a value, ``None``, or omit them) without + restating the shared ``tool_name``/``tool_args``/client fields. + """ payload = { "tool_name": "dummy", "tool_args": {"a": 1}, @@ -735,8 +749,14 @@ async def test_report_tool_usage_success(mock_track, client: AsyncClient): "client_version": "1.0", "protocol_version": "1.1", } - headers = {"User-Agent": "BlockscoutMCP/0.0"} - response = await client.post("/v1/report_tool_usage", json=payload, headers=headers) + payload.update(overrides) + return await client.post("/v1/report_tool_usage", json=payload, headers={"User-Agent": _REPORT_USER_AGENT}) + + +@pytest.mark.asyncio +@patch("blockscout_mcp_server.api.routes.analytics.track_community_usage") +async def test_report_tool_usage_success(mock_track, client: AsyncClient): + response = await post_report(client, auth_origin="client", api_key_fingerprint="a" * 64) assert response.status_code == 202 mock_track.assert_called_once() _, kwargs = mock_track.call_args @@ -745,7 +765,9 @@ async def test_report_tool_usage_success(mock_track, client: AsyncClient): assert kwargs["report"].client_name == "cli" assert kwargs["report"].client_version == "1.0" assert kwargs["report"].protocol_version == "1.1" - assert kwargs["user_agent"] == headers["User-Agent"] + assert kwargs["report"].auth_origin == "client" + assert kwargs["report"].api_key_fingerprint == "a" * 64 + assert kwargs["user_agent"] == _REPORT_USER_AGENT @pytest.mark.asyncio @@ -767,6 +789,90 @@ async def test_report_tool_usage_missing_header(client: AsyncClient): assert response.status_code == 400 +@pytest.mark.asyncio +@patch("blockscout_mcp_server.api.routes.analytics.track_community_usage") +async def test_report_tool_usage_legacy_payload_without_new_fields(mock_track, client: AsyncClient): + """A legacy payload that omits auth_origin/api_key_fingerprint is still accepted. + + The analytics layer renders a None auth_origin as 'unknown', so the forwarded report + must carry None rather than being rejected outright. + """ + response = await post_report(client) + assert response.status_code == 202 + mock_track.assert_called_once() + _, kwargs = mock_track.call_args + assert kwargs["report"].auth_origin is None + + +@pytest.mark.asyncio +@patch("blockscout_mcp_server.api.routes.analytics.track_community_usage") +async def test_report_tool_usage_tolerates_unrecognized_auth_origin(mock_track, client: AsyncClient): + """An unrecognized auth_origin does not drop the otherwise-valid report. + + The value this receiver does not recognize is coerced to None by the model (rendered as + "unknown" downstream), so the report is still accepted (202) and forwarded once with + `auth_origin is None`. This keeps a version-skewed community reporter reporting instead of + silently losing 100% of its telemetry to a 422 that the fire-and-forget sender never sees. + """ + response = await post_report(client, auth_origin="bogus") + assert response.status_code == 202 + mock_track.assert_called_once() + _, kwargs = mock_track.call_args + assert kwargs["report"].auth_origin is None + + +@pytest.mark.asyncio +@patch("blockscout_mcp_server.api.routes.analytics.track_community_usage") +async def test_report_tool_usage_tolerates_malformed_fingerprint(mock_track, client: AsyncClient): + """A syntactically invalid api_key_fingerprint does not drop the otherwise-valid report. + + The not-yet-consumed fingerprint is coerced to None by the model, so the report is still + accepted (202) and forwarded once with `api_key_fingerprint is None`. + """ + response = await post_report(client, api_key_fingerprint="not-a-hash") + assert response.status_code == 202 + mock_track.assert_called_once() + _, kwargs = mock_track.call_args + assert kwargs["report"].api_key_fingerprint is None + + +@pytest.mark.asyncio +@patch("blockscout_mcp_server.api.routes.analytics.track_community_usage") +async def test_report_tool_usage_explicit_none_auth_origin_string_with_null_fingerprint( + mock_track, client: AsyncClient +): + """The exact no-key wire shape an updated reporter sends: auth_origin="none" with a null + fingerprint, sent explicitly rather than omitted. + + This guards against the endpoint rejecting real no-key reports from updated reporters. + """ + response = await post_report(client, auth_origin="none", api_key_fingerprint=None) + assert response.status_code == 202 + mock_track.assert_called_once() + _, kwargs = mock_track.call_args + assert kwargs["report"].auth_origin == "none" + assert kwargs["report"].api_key_fingerprint is None + + +@pytest.mark.asyncio +@patch("blockscout_mcp_server.api.routes.analytics.track_community_usage") +async def test_report_tool_usage_explicit_null_auth_origin_with_null_fingerprint(mock_track, client: AsyncClient): + """A JSON null auth_origin (not the string "none") with a null fingerprint is accepted. + + Phase 4 adds both keys to the outbound payload unconditionally, so any caller that omits + the auth_origin keyword causes the sender to serialize a literal `"auth_origin": null` on + the wire. This is the HTTP-boundary counterpart of the Phase 1 explicit-auth_origin=None + model test, and it is the only route case that catches a `Literal[...]`-without-`| None` + typing regression, which would otherwise 422 a real report. + """ + response = await post_report(client, auth_origin=None, api_key_fingerprint=None) + assert response.status_code == 202 + mock_track.assert_called_once() + _, kwargs = mock_track.call_args + assert kwargs["report"].auth_origin is None + assert kwargs["report"].api_key_fingerprint is None + + @pytest.mark.asyncio @patch("blockscout_mcp_server.api.routes.direct_api_call", new_callable=AsyncMock) async def test_direct_api_call_post_success(mock_tool, client: AsyncClient): diff --git a/tests/pro_api_key_helpers.py b/tests/pro_api_key_helpers.py new file mode 100644 index 00000000..ec517fa3 --- /dev/null +++ b/tests/pro_api_key_helpers.py @@ -0,0 +1,45 @@ +# SPDX-License-Identifier: LicenseRef-Blockscout +"""Shared builders for MCP-like request contexts used across PRO API key tests. + +Several test modules independently reconstructed the same request-context shape +to exercise client PRO API key extraction. Centralizing the two flavors here +keeps them in one place and makes the header-encoding contract explicit: + +- :func:`ctx_with_header` uses real :class:`starlette.datastructures.Headers` + and passes the header name in non-canonical (upper) casing so the + case-insensitive lookup path is exercised. Use it for well-formed values. +- :func:`ctx_with_malformed_header` uses a plain ``dict`` so a value that real + starlette ``Headers`` would refuse to latin-1 encode (control characters, + over-length) can still be injected; extraction only needs a case-insensitive + ``Mapping`` lookup. +""" + +from __future__ import annotations + +from types import SimpleNamespace + +from starlette.datastructures import Headers + + +def ctx_with_header(header_name: str, header_value: str) -> SimpleNamespace: + """Build a minimal MCP-like context carrying *header_value* under *header_name*. + + Real :class:`starlette.datastructures.Headers` are used with the header name + upper-cased so the case-insensitive lookup path is exercised. + """ + headers = Headers(headers={header_name.upper(): header_value}) + request = SimpleNamespace(headers=headers) + return SimpleNamespace(request_context=SimpleNamespace(request=request)) + + +def ctx_with_malformed_header(header_name: str, header_value: str) -> SimpleNamespace: + """Build a context whose header value bypasses starlette's encoding checks. + + A plain ``dict`` is used so a value real :class:`starlette.datastructures.Headers` + would refuse to encode (control characters, over-length) can still be + injected. The header name keeps its given casing; extraction is + case-insensitive regardless. + """ + headers = {header_name: header_value} + request = SimpleNamespace(headers=headers) + return SimpleNamespace(request_context=SimpleNamespace(request=request)) diff --git a/tests/test_analytics.py b/tests/test_analytics.py index ec3bffa3..f243804a 100644 --- a/tests/test_analytics.py +++ b/tests/test_analytics.py @@ -145,6 +145,42 @@ def test_tracks_with_intermediary_no_client_or_user_agent(monkeypatch): assert args[2]["client_name"] == "N/A/ClaudeDesktop" +def test_tracks_threaded_auth_origin_into_property_bag(monkeypatch): + """The pre-computed ``auth_origin`` threaded by the caller reaches the property bag verbatim.""" + monkeypatch.setattr(server_config, "mixpanel_token", "test-token", raising=False) + headers = {"x-forwarded-for": "203.0.113.5", "user-agent": "pytest-UA"} + req = DummyRequest(headers=headers) + ctx = DummyCtx(request=req, client_name="clientA", client_version="1.0.0") + with patch("blockscout_mcp_server.analytics.Mixpanel") as mp_cls: + mp_instance = MagicMock() + mp_cls.return_value = mp_instance + analytics.set_http_mode(True) + analytics.track_tool_invocation(ctx, "tool_name", {"x": 2}, auth_origin="server") + args, _ = mp_instance.track.call_args + assert args[2]["auth_origin"] == "server" + + +def test_tracks_auth_origin_unknown_when_not_threaded(monkeypatch): + """When no ``auth_origin`` is threaded (``None``), the sink records the ``unknown`` sentinel. + + The sink never re-derives the origin from ``ctx`` — doing so would re-run the + computation that ``telemetry.resolve_auth_signals`` already guarded and, if it + fails, drop the whole Mixpanel event. Mirrors ``track_community_usage``. + """ + monkeypatch.setattr(server_config, "mixpanel_token", "test-token", raising=False) + monkeypatch.setattr(server_config, "pro_api_key", "server-secret", raising=False) + headers = {"x-forwarded-for": "203.0.113.5", "user-agent": "pytest-UA"} + req = DummyRequest(headers=headers) + ctx = DummyCtx(request=req, client_name="clientA", client_version="1.0.0") + with patch("blockscout_mcp_server.analytics.Mixpanel") as mp_cls: + mp_instance = MagicMock() + mp_cls.return_value = mp_instance + analytics.set_http_mode(True) + analytics.track_tool_invocation(ctx, "tool_name", {"x": 2}) + args, _ = mp_instance.track.call_args + assert args[2]["auth_origin"] == "unknown" + + def test_track_event_tracks_when_enabled(monkeypatch): monkeypatch.setattr(server_config, "mixpanel_token", "test-token", raising=False) req = DummyRequest(headers={"user-agent": "pytest-UA"}, host="203.0.113.5") @@ -193,7 +229,7 @@ def test_pro_api_key_not_in_analytics_payload(monkeypatch): mp_instance = MagicMock() mp_cls.return_value = mp_instance analytics.set_http_mode(True) - analytics.track_tool_invocation(ctx, "some_tool", {"x": 1}) + analytics.track_tool_invocation(ctx, "some_tool", {"x": 1}, auth_origin="client") assert mp_instance.track.called call_args = mp_instance.track.call_args @@ -210,6 +246,10 @@ def test_pro_api_key_not_in_analytics_payload(monkeypatch): assert client_key not in str(properties) assert client_key not in str(kwargs) + # The caller threaded auth_origin='client'; it reaches the bag but the key never does. + assert properties.get("auth_origin") == "client" + assert "api_key_fingerprint" not in properties + def test_pro_api_key_not_in_analytics_payload_rest_source(monkeypatch): """The client-supplied PRO API key must not appear in the Mixpanel payload on the REST path. @@ -237,7 +277,7 @@ def test_pro_api_key_not_in_analytics_payload_rest_source(monkeypatch): mp_instance = MagicMock() mp_cls.return_value = mp_instance analytics.set_http_mode(True) - analytics.track_tool_invocation(ctx, "some_tool", {"x": 1}) + analytics.track_tool_invocation(ctx, "some_tool", {"x": 1}, auth_origin="client") assert mp_instance.track.called call_args = mp_instance.track.call_args @@ -257,6 +297,10 @@ def test_pro_api_key_not_in_analytics_payload_rest_source(monkeypatch): # Confirm the source is correctly reported as 'rest' assert properties.get("source") == "rest" + # The caller threaded auth_origin='client'; it reaches the bag but the key never does. + assert properties.get("auth_origin") == "client" + assert "api_key_fingerprint" not in properties + def test_track_community_usage(monkeypatch): monkeypatch.setattr(server_config, "mixpanel_token", "test-token", raising=False) @@ -283,6 +327,83 @@ def test_track_community_usage(monkeypatch): assert kwargs.get("meta") == {"ip": "203.0.113.5"} +def test_track_community_usage_auth_origin_from_report(monkeypatch): + """auth_origin is forwarded verbatim from the report when present.""" + monkeypatch.setattr(server_config, "mixpanel_token", "test-token", raising=False) + with patch("blockscout_mcp_server.analytics.Mixpanel") as mp_cls: + mp_instance = MagicMock() + mp_cls.return_value = mp_instance + analytics.set_http_mode(True) + report = ToolUsageReport( + tool_name="foo", + tool_args={"a": 1}, + client_name="cli", + client_version="1.0", + protocol_version="1.1", + auth_origin="server", + ) + analytics.track_community_usage(report, ip="203.0.113.5", user_agent="ua") + args, _ = mp_instance.track.call_args + properties = args[2] + assert properties["auth_origin"] == "server" + + +def test_track_community_usage_auth_origin_defaults_to_unknown(monkeypatch): + """A legacy report with no auth_origin maps to the 'unknown' sentinel.""" + monkeypatch.setattr(server_config, "mixpanel_token", "test-token", raising=False) + with patch("blockscout_mcp_server.analytics.Mixpanel") as mp_cls: + mp_instance = MagicMock() + mp_cls.return_value = mp_instance + analytics.set_http_mode(True) + report = ToolUsageReport( + tool_name="foo", + tool_args={"a": 1}, + client_name="cli", + client_version="1.0", + protocol_version="1.1", + auth_origin=None, + ) + analytics.track_community_usage(report, ip="203.0.113.5", user_agent="ua") + args, _ = mp_instance.track.call_args + properties = args[2] + assert properties["auth_origin"] == "unknown" + + +def test_track_community_usage_fingerprint_never_reaches_mixpanel(monkeypatch): + """The api_key_fingerprint must not leak anywhere in the Mixpanel call. + + Checks the entire call (distinct_id, event name, properties, meta) for the + fingerprint value, not merely the `properties["api_key_fingerprint"]` key + -- this also catches a leak via distinct_id or a differently named property. + Also asserts distinct_id is unaffected by the fingerprint (still derived + only from ip/client_name/client_version), proving identity is not yet + strengthened by it. + """ + monkeypatch.setattr(server_config, "mixpanel_token", "test-token", raising=False) + distinctive_fingerprint = "ab" * 32 # 64-char lowercase hex, easy to spot in a leak + with patch("blockscout_mcp_server.analytics.Mixpanel") as mp_cls: + mp_instance = MagicMock() + mp_cls.return_value = mp_instance + analytics.set_http_mode(True) + report = ToolUsageReport( + tool_name="foo", + tool_args={"a": 1}, + client_name="cli", + client_version="1.0", + protocol_version="1.1", + auth_origin="client", + api_key_fingerprint=distinctive_fingerprint, + ) + analytics.track_community_usage(report, ip="203.0.113.5", user_agent="ua") + mp_instance.track.assert_called_once() + call_args = mp_instance.track.call_args + assert distinctive_fingerprint not in str(call_args) + + args, _ = call_args + expected_distinct_id = analytics._build_distinct_id("203.0.113.5", report.client_name, report.client_version) + assert args[0] == expected_distinct_id + + # --------------------------------------------------------------------------- # track_resource_read tests # --------------------------------------------------------------------------- @@ -309,7 +430,11 @@ def test_track_resource_read_emits_correct_event(monkeypatch): """When enabled, mp.track is called with RESOURCE_READ event and uri in tool_args.""" monkeypatch.setattr(server_config, "mixpanel_token", "test-token", raising=False) uri = "blockscout-mcp://skill/SKILL.md" - headers = {"x-forwarded-for": "203.0.113.5", "user-agent": "pytest-UA"} + headers = { + "x-forwarded-for": "203.0.113.5", + "user-agent": "pytest-UA", + "Blockscout-MCP-Pro-Api-Key": "client-secret", + } req = DummyRequest(headers=headers) ctx = DummyCtx(request=req, client_name="clientA", client_version="1.0.0") with patch("blockscout_mcp_server.analytics.Mixpanel") as mp_cls: @@ -320,6 +445,7 @@ def test_track_resource_read_emits_correct_event(monkeypatch): ctx, uri, client_meta=ClientMeta(name="clientA", version="1.0.0", protocol="2024-11-05", user_agent="pytest-UA"), + auth_origin="client", ) mp_instance.track.assert_called_once() args, kwargs = mp_instance.track.call_args @@ -331,3 +457,6 @@ def test_track_resource_read_emits_correct_event(monkeypatch): assert properties["protocol_version"] == "2024-11-05" assert properties["ip"] == "203.0.113.5" assert "source" in properties + # track_resource_read threads the caller's auth_origin straight through to + # track_tool_invocation, so the value provided here reaches the property bag. + assert properties["auth_origin"] == "client" diff --git a/tests/test_observability.py b/tests/test_observability.py index da12ff2a..eb8642d6 100644 --- a/tests/test_observability.py +++ b/tests/test_observability.py @@ -8,6 +8,7 @@ from pydantic import AnyUrl +from blockscout_mcp_server import pro_api_key_context from blockscout_mcp_server.client_meta import ( UNDEFINED_CLIENT_NAME, UNDEFINED_CLIENT_VERSION, @@ -83,7 +84,7 @@ def test_uri_normalisation_anyurl_becomes_str(): ctx = _make_ctx() captured: list[str] = [] - def fake_track(ctx_, uri_, client_meta=None): + def fake_track(ctx_, uri_, client_meta=None, auth_origin=None): captured.append(uri_) with ( @@ -120,6 +121,40 @@ def test_fan_out_both_sinks_called(): mock_create_task.assert_called_once() +def test_community_sink_forwards_auth_origin_and_fingerprint(monkeypatch): + """send_community_resource_report receives auth_origin/api_key_fingerprint derived from ctx.""" + # Keep community telemetry enabled so telemetry.resolve_auth_signals derives the signals instead + # of short-circuiting to (None, None) when BLOCKSCOUT_DISABLE_COMMUNITY_TELEMETRY is set ambiently. + monkeypatch.setattr("blockscout_mcp_server.telemetry.config.disable_community_telemetry", False, raising=False) + monkeypatch.setattr("blockscout_mcp_server.pro_api_key_context.config.pro_api_key", "server-key", raising=False) + ctx = _make_ctx() + + # new_callable=MagicMock: send_community_resource_report is an async def, so a bare + # patch() would auto-create an AsyncMock whose call yields an (orphaned) coroutine + # when create_task is also mocked away. See the module-level idiom note above. + with ( + patch("blockscout_mcp_server.observability.analytics.track_resource_read"), + patch( + "blockscout_mcp_server.observability.telemetry.send_community_resource_report", + new_callable=MagicMock, + ) as mock_send_report, + patch("blockscout_mcp_server.observability.asyncio.create_task") as mock_create_task, + ): + log_resource_read(_SKILL_URI, ctx) + + mock_create_task.assert_called_once() + # The coroutine handed to create_task is the one produced by calling the mocked sink. + mock_send_report.assert_called_once() + call_kwargs = mock_send_report.call_args.kwargs + assert call_kwargs["auth_origin"] == "server" + # Pin the forwarded fingerprint against the production helper (not merely "not None"), so a + # regression that forwards a different hash here — an unprefixed digest or the client-key hash — + # is caught. This boundary asserts wiring; the fingerprint *scheme* is re-derived independently + # only in the helper's own unit tests (_expected_fingerprint), so a scheme change touches one place. + expected_fingerprint = pro_api_key_context._fingerprint_pro_api_key("server-key") + assert call_kwargs["api_key_fingerprint"] == expected_fingerprint + + # --------------------------------------------------------------------------- # Error swallowing — no exception propagates # --------------------------------------------------------------------------- diff --git a/tests/test_pro_api_key_context.py b/tests/test_pro_api_key_context.py index 089ce26a..83118e11 100644 --- a/tests/test_pro_api_key_context.py +++ b/tests/test_pro_api_key_context.py @@ -9,7 +9,7 @@ from unittest.mock import MagicMock import pytest -from starlette.datastructures import Headers +from pro_api_key_helpers import ctx_with_header, ctx_with_malformed_header from blockscout_mcp_server.config import config from blockscout_mcp_server.pro_api_key_context import ( @@ -108,23 +108,10 @@ def test_normalize_exactly_max_length_is_valid(): # =========================================================================== -def _mcp_ctx_with_header(header_name: str, header_value: str) -> SimpleNamespace: - """Build a minimal MCP-like context carrying *header_value* under *header_name*. - - Uses real ``starlette.datastructures.Headers`` with non-canonical casing to - exercise case-insensitive lookup. - """ - # Use non-canonical casing to prove case-insensitive lookup works. - # starlette.datastructures.Headers accepts a Mapping (dict) for the headers param. - headers = Headers(headers={header_name.upper(): header_value}) - request = SimpleNamespace(headers=headers) - return SimpleNamespace(request_context=SimpleNamespace(request=request)) - - def test_extraction_rest_call_source_reads_header(monkeypatch): """A REST-source context that carries the configured header must yield _Valid.""" monkeypatch.setattr(config, "pro_api_key_header", "Blockscout-MCP-Pro-Api-Key", raising=False) - ctx = _mcp_ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-123") + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-123") ctx.call_source = "rest" # type: ignore[attr-defined] state = extract_client_pro_api_key_from_ctx(ctx) assert isinstance(state, _Valid) @@ -134,7 +121,7 @@ def test_extraction_rest_call_source_reads_header(monkeypatch): def test_extraction_rest_call_source_absent_header_is_absent(monkeypatch): """A REST-source context with no header value yields _Absent.""" monkeypatch.setattr(config, "pro_api_key_header", "Blockscout-MCP-Pro-Api-Key", raising=False) - ctx = _mcp_ctx_with_header("Blockscout-MCP-Pro-Api-Key", "") + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "") ctx.call_source = "rest" # type: ignore[attr-defined] assert isinstance(extract_client_pro_api_key_from_ctx(ctx), _Absent) @@ -142,34 +129,30 @@ def test_extraction_rest_call_source_absent_header_is_absent(monkeypatch): def test_extraction_rest_call_source_malformed_header_is_malformed(monkeypatch): """A REST-source context with a control-char header value yields _Malformed.""" monkeypatch.setattr(config, "pro_api_key_header", "Blockscout-MCP-Pro-Api-Key", raising=False) - # Use a plain dict so we can inject a control-character value that starlette - # would refuse to encode. - headers = {"Blockscout-MCP-Pro-Api-Key": "bad\nkey"} - request = SimpleNamespace(headers=headers) - ctx = SimpleNamespace(request_context=SimpleNamespace(request=request), call_source="rest") + ctx = ctx_with_malformed_header("Blockscout-MCP-Pro-Api-Key", "bad\nkey") + ctx.call_source = "rest" assert isinstance(extract_client_pro_api_key_from_ctx(ctx), _Malformed) def test_extraction_rest_call_source_over_length_header_is_malformed(monkeypatch): """A REST-source context with an over-length header value yields _Malformed.""" monkeypatch.setattr(config, "pro_api_key_header", "Blockscout-MCP-Pro-Api-Key", raising=False) - headers = {"Blockscout-MCP-Pro-Api-Key": "a" * 257} - request = SimpleNamespace(headers=headers) - ctx = SimpleNamespace(request_context=SimpleNamespace(request=request), call_source="rest") + ctx = ctx_with_malformed_header("Blockscout-MCP-Pro-Api-Key", "a" * 257) + ctx.call_source = "rest" assert isinstance(extract_client_pro_api_key_from_ctx(ctx), _Malformed) def test_extraction_rest_call_source_disabled_feature_is_absent(monkeypatch): """Feature disabled (empty header config) → absent even if the header is present.""" monkeypatch.setattr(config, "pro_api_key_header", "", raising=False) - ctx = _mcp_ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-123") + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-123") ctx.call_source = "rest" # type: ignore[attr-defined] assert isinstance(extract_client_pro_api_key_from_ctx(ctx), _Absent) def test_extraction_empty_header_config_is_absent(monkeypatch): monkeypatch.setattr(config, "pro_api_key_header", "", raising=False) - ctx = _mcp_ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-123") + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-123") assert isinstance(extract_client_pro_api_key_from_ctx(ctx), _Absent) @@ -194,11 +177,8 @@ def test_extraction_stdio_like_no_request_is_absent(monkeypatch): def test_extraction_mcp_ctx_with_valid_header(monkeypatch): """Real starlette Headers + non-canonical casing → valid state.""" monkeypatch.setattr(config, "pro_api_key_header", "Blockscout-MCP-Pro-Api-Key", raising=False) - # Pass the header under ALL-CAPS to prove case-insensitive lookup. - # starlette.datastructures.Headers accepts a Mapping (dict) for the headers param. - headers = Headers(headers={"BLOCKSCOUT-MCP-PRO-API-KEY": "my-client-key"}) - request = SimpleNamespace(headers=headers) - ctx = SimpleNamespace(request_context=SimpleNamespace(request=request)) + # ctx_with_header upper-cases the header name, so this exercises case-insensitive lookup. + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "my-client-key") state = extract_client_pro_api_key_from_ctx(ctx) assert isinstance(state, _Valid) @@ -271,10 +251,7 @@ async def test_malformed_error_does_not_embed_control_char_value(monkeypatch): async def dummy(ctx) -> None: resolve_pro_api_key() - # Plain dict headers: real starlette Headers reject control-char values at - # construction time, while the extractor works with any Mapping. - request = SimpleNamespace(headers={"Blockscout-MCP-Pro-Api-Key": raw_value}) - ctx = SimpleNamespace(request_context=SimpleNamespace(request=request)) + ctx = ctx_with_malformed_header("Blockscout-MCP-Pro-Api-Key", raw_value) with pytest.raises(ValueError) as exc_info: await dummy(ctx=ctx) @@ -295,8 +272,7 @@ async def test_malformed_error_does_not_embed_over_length_value(monkeypatch): async def dummy(ctx) -> None: resolve_pro_api_key() - request = SimpleNamespace(headers={"Blockscout-MCP-Pro-Api-Key": raw_value}) - ctx = SimpleNamespace(request_context=SimpleNamespace(request=request)) + ctx = ctx_with_malformed_header("Blockscout-MCP-Pro-Api-Key", raw_value) with pytest.raises(ValueError) as exc_info: await dummy(ctx=ctx) @@ -308,16 +284,6 @@ async def dummy(ctx) -> None: # =========================================================================== -def _ctx_with_starlette_headers(header_name: str, header_value: str) -> SimpleNamespace: - """Build an MCP-like context using real starlette.datastructures.Headers (dict form). - - The header name is passed in UPPER CASE so the case-insensitive lookup is exercised. - """ - headers = Headers(headers={header_name.upper(): header_value}) - request = SimpleNamespace(headers=headers) - return SimpleNamespace(request_context=SimpleNamespace(request=request)) - - @pytest.mark.asyncio async def test_decorator_sets_valid_state_during_call(monkeypatch): monkeypatch.setattr(config, "pro_api_key_header", "Blockscout-MCP-Pro-Api-Key", raising=False) @@ -328,7 +294,7 @@ async def test_decorator_sets_valid_state_during_call(monkeypatch): async def dummy(ctx) -> None: observed_state.append(_client_key_state.get()) - ctx = _ctx_with_starlette_headers("Blockscout-MCP-Pro-Api-Key", "client-key-xyz") + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-xyz") await dummy(ctx=ctx) @@ -345,7 +311,7 @@ async def test_decorator_resets_state_after_call(monkeypatch): async def dummy(ctx) -> None: pass - ctx = _ctx_with_starlette_headers("Blockscout-MCP-Pro-Api-Key", "client-key-xyz") + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-xyz") await dummy(ctx=ctx) @@ -361,7 +327,7 @@ async def test_decorator_resets_state_even_when_wrapped_function_raises(monkeypa async def dummy(ctx) -> None: raise RuntimeError("boom") - ctx = _ctx_with_starlette_headers("Blockscout-MCP-Pro-Api-Key", "client-key-xyz") + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-xyz") with pytest.raises(RuntimeError, match="boom"): await dummy(ctx=ctx) @@ -381,7 +347,7 @@ async def test_decorator_rest_call_source_sets_valid_state(monkeypatch): async def dummy(ctx) -> None: observed_state.append(_client_key_state.get()) - ctx = _ctx_with_starlette_headers("Blockscout-MCP-Pro-Api-Key", "client-key-xyz") + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "client-key-xyz") ctx.call_source = "rest" # type: ignore[attr-defined] await dummy(ctx=ctx) @@ -407,12 +373,7 @@ async def test_decorator_malformed_does_not_raise_from_decorator(monkeypatch): async def dummy(ctx) -> None: ran.append(True) - # Use a plain dict so we can inject a control-character value that starlette - # would refuse to encode — the extractor just calls get_header_case_insensitive - # which works with any Mapping. - headers = {"Blockscout-MCP-Pro-Api-Key": "bad\x00key"} - request = SimpleNamespace(headers=headers) - ctx = SimpleNamespace(request_context=SimpleNamespace(request=request)) + ctx = ctx_with_malformed_header("Blockscout-MCP-Pro-Api-Key", "bad\x00key") # Must not raise at decoration time or call time (malformed raise is in resolve_pro_api_key) await dummy(ctx=ctx) @@ -449,13 +410,8 @@ async def dummy(ctx, task_name: str) -> None: await asyncio.sleep(0) results[task_name] = resolve_pro_api_key() - def _ctx_with_key(key: str) -> SimpleNamespace: - headers = Headers(headers={"Blockscout-MCP-Pro-Api-Key": key}) - request = SimpleNamespace(headers=headers) - return SimpleNamespace(request_context=SimpleNamespace(request=request)) - - ctx_a = _ctx_with_key("key-for-task-a") - ctx_b = _ctx_with_key("key-for-task-b") + ctx_a = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "key-for-task-a") + ctx_b = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "key-for-task-b") await asyncio.gather( dummy(ctx=ctx_a, task_name="a"), diff --git a/tests/test_pro_api_key_context_auth_signals.py b/tests/test_pro_api_key_context_auth_signals.py new file mode 100644 index 00000000..5466becd --- /dev/null +++ b/tests/test_pro_api_key_context_auth_signals.py @@ -0,0 +1,234 @@ +# SPDX-License-Identifier: LicenseRef-Blockscout +"""Unit tests for the ctx-derived auth-origin and fingerprint helpers. + +Kept in a focused sibling module rather than grown into +``tests/test_pro_api_key_context.py`` (already 466 LOC, close to the +500-LOC test-file limit). +""" + +from __future__ import annotations + +import hashlib +from unittest.mock import MagicMock + +import pytest +from pro_api_key_helpers import ctx_with_header, ctx_with_malformed_header + +from blockscout_mcp_server import pro_api_key_context +from blockscout_mcp_server.config import config +from blockscout_mcp_server.constants import PRO_API_KEY_HASH_PREFIX +from blockscout_mcp_server.pro_api_key_context import ( + compute_auth_signals, + extract_client_pro_api_key_from_ctx, + resolve_pro_api_key, +) + +_HEADER_NAME = "Blockscout-MCP-Pro-Api-Key" + + +def _expected_fingerprint(key: str) -> str: + """Compute the expected fingerprint via the same UTF-8 byte construction.""" + return hashlib.sha256(f"{PRO_API_KEY_HASH_PREFIX}{key}".encode()).hexdigest() + + +# =========================================================================== +# Fingerprint safety: raw key never leaks; prefix actually participates +# =========================================================================== + + +def test_fingerprint_never_equals_or_contains_raw_key(monkeypatch): + monkeypatch.setattr(config, "pro_api_key_header", _HEADER_NAME, raising=False) + monkeypatch.setattr(config, "pro_api_key", "", raising=False) + raw_key = "super-secret-client-key-456" + ctx = ctx_with_header(_HEADER_NAME, raw_key) + fingerprint = compute_auth_signals(ctx)[1] + assert fingerprint is not None + assert fingerprint != raw_key + assert raw_key not in fingerprint + + +def test_fingerprint_prefix_actually_participates(monkeypatch): + """The result must differ from a hash computed without the domain-separation prefix.""" + monkeypatch.setattr(config, "pro_api_key_header", _HEADER_NAME, raising=False) + monkeypatch.setattr(config, "pro_api_key", "", raising=False) + raw_key = "client-key-789" + ctx = ctx_with_header(_HEADER_NAME, raw_key) + fingerprint = compute_auth_signals(ctx)[1] + unprefixed_hash = hashlib.sha256(raw_key.encode("utf-8")).hexdigest() + assert fingerprint != unprefixed_hash + + +# =========================================================================== +# compute_auth_signals — the single source of truth for both signals +# =========================================================================== +# +# These cases pin the (origin, fingerprint) pairing per branch so the two signals +# can never silently diverge. A server key is configured throughout so a +# server-fallback regression cannot hide: a valid client header must still win and +# a malformed one must still yield no usable key (no silent fallback to the server +# key), mirroring the precedence enforced by resolve_pro_api_key(). + + +def test_auth_signals_valid_client_returns_client_and_client_hash(monkeypatch): + monkeypatch.setattr(config, "pro_api_key_header", _HEADER_NAME, raising=False) + monkeypatch.setattr(config, "pro_api_key", "server-key", raising=False) + ctx = ctx_with_header(_HEADER_NAME, "client-key-123") + assert compute_auth_signals(ctx) == ("client", _expected_fingerprint("client-key-123")) + + +def test_auth_signals_malformed_returns_none_and_no_fingerprint(monkeypatch): + monkeypatch.setattr(config, "pro_api_key_header", _HEADER_NAME, raising=False) + monkeypatch.setattr(config, "pro_api_key", "server-key", raising=False) + ctx = ctx_with_malformed_header(_HEADER_NAME, "bad\nkey") + assert compute_auth_signals(ctx) == ("none", None) + + +def test_auth_signals_absent_with_server_key_returns_server_and_server_hash(monkeypatch): + monkeypatch.setattr(config, "pro_api_key_header", _HEADER_NAME, raising=False) + monkeypatch.setattr(config, "pro_api_key", "server-key", raising=False) + ctx = ctx_with_header(_HEADER_NAME, "") + assert compute_auth_signals(ctx) == ("server", _expected_fingerprint("server-key")) + + +def test_auth_signals_absent_with_no_server_key_returns_none_and_no_fingerprint(monkeypatch): + monkeypatch.setattr(config, "pro_api_key_header", _HEADER_NAME, raising=False) + monkeypatch.setattr(config, "pro_api_key", "", raising=False) + ctx = ctx_with_header(_HEADER_NAME, "") + assert compute_auth_signals(ctx) == ("none", None) + + +# =========================================================================== +# Server-key fingerprint memoization — the constant server hash is computed once +# =========================================================================== +# +# The server key is fixed for the process lifetime, so its SHA-256 is memoized +# (see _server_api_key_fingerprint) and the observability hot path reuses the one +# digest instead of re-hashing the same constant on every tool call / resource read. + + +def test_server_fingerprint_is_memoized_across_calls(monkeypatch): + """Absent client key + server key -> ("server", ), hashing the server key once. + + Pins the optimization that replaces the old ``include_server_fingerprint`` + gate: rather than skipping the hash when no consumer exists, the fixed server + key is hashed once and reused. The spy below proves a second derivation reuses + the memoized digest instead of re-running the SHA-256. + """ + monkeypatch.setattr(config, "pro_api_key_header", _HEADER_NAME, raising=False) + monkeypatch.setattr(config, "pro_api_key", "server-key", raising=False) + pro_api_key_context._server_api_key_fingerprint.cache_clear() + + fingerprint_spy = MagicMock(side_effect=pro_api_key_context._fingerprint_pro_api_key) + monkeypatch.setattr(pro_api_key_context, "_fingerprint_pro_api_key", fingerprint_spy) + + ctx = ctx_with_header(_HEADER_NAME, "") # absent client key -> server fallback + expected = ("server", _expected_fingerprint("server-key")) + assert compute_auth_signals(ctx) == expected + assert compute_auth_signals(ctx) == expected + # Two derivations, but the constant server key was hashed exactly once. + fingerprint_spy.assert_called_once_with("server-key") + + +def test_client_fingerprint_is_never_memoized(monkeypatch): + """A per-request client key is re-hashed each call — only the server key is cached. + + The client-key path must never route through the memoizing helper: caching a + per-request secret would both retain it and let a stale digest leak across + requests. Two calls with a valid client header therefore hash it twice. + """ + monkeypatch.setattr(config, "pro_api_key_header", _HEADER_NAME, raising=False) + monkeypatch.setattr(config, "pro_api_key", "", raising=False) + + fingerprint_spy = MagicMock(side_effect=pro_api_key_context._fingerprint_pro_api_key) + monkeypatch.setattr(pro_api_key_context, "_fingerprint_pro_api_key", fingerprint_spy) + + ctx = ctx_with_header(_HEADER_NAME, "client-key-123") + expected = ("client", _expected_fingerprint("client-key-123")) + assert compute_auth_signals(ctx) == expected + assert compute_auth_signals(ctx) == expected + assert fingerprint_spy.call_count == 2 + + +# =========================================================================== +# Coupling invariant: auth_origin must never disagree with resolve_pro_api_key +# =========================================================================== +# +# Both paths delegate precedence to _apply_key_precedence, so the reported origin +# and the key resolve_pro_api_key() actually uses are two views of one decision. +# This pins the end-to-end invariant directly (not just the shared helper): if a +# future edit re-introduces a divergent branch in either wrapper — the drift risk +# #423 exists to prevent — one of these cases fails. compute_auth_signals is +# driven from ctx headers; resolve_pro_api_key from the ContextVar set to the +# *same* state extracted from that ctx, so both see one logical input. + + +def _resolve_outcome(state: pro_api_key_context.ClientKeyState) -> tuple[str, str | None]: + """Run ``resolve_pro_api_key`` with the ContextVar set to *state*; report its outcome. + + Returns ``("key", )`` for a returned key (possibly ``""``) or + ``("raised", None)`` when the malformed-key ``ValueError`` is raised. + """ + token = pro_api_key_context._client_key_state.set(state) + try: + return "key", resolve_pro_api_key() + except ValueError: + return "raised", None + finally: + pro_api_key_context._client_key_state.reset(token) + + +@pytest.mark.parametrize( + ("server_key", "make_ctx", "expected_origin", "expected_resolution"), + [ + pytest.param( + "server-key", + lambda: ctx_with_header(_HEADER_NAME, "client-key-123"), + "client", + ("key", "client-key-123"), + id="valid-client-wins", + ), + pytest.param( + "server-key", + lambda: ctx_with_malformed_header(_HEADER_NAME, "bad\nkey"), + "none", + ("raised", None), + id="malformed-rejected-no-fallback", + ), + pytest.param( + "server-key", + lambda: ctx_with_header(_HEADER_NAME, ""), + "server", + ("key", "server-key"), + id="absent-falls-back-to-server", + ), + pytest.param( + "", + lambda: ctx_with_header(_HEADER_NAME, ""), + "none", + ("key", ""), + id="absent-no-server-key", + ), + ], +) +def test_auth_origin_stays_consistent_with_resolve_pro_api_key( + monkeypatch, server_key, make_ctx, expected_origin, expected_resolution +): + monkeypatch.setattr(config, "pro_api_key_header", _HEADER_NAME, raising=False) + monkeypatch.setattr(config, "pro_api_key", server_key, raising=False) + pro_api_key_context._server_api_key_fingerprint.cache_clear() + + ctx = make_ctx() + origin, _fingerprint = compute_auth_signals(ctx) + assert origin == expected_origin + + # resolve sees the SAME state the telemetry path derived from ctx. + state = extract_client_pro_api_key_from_ctx(ctx) + resolution = _resolve_outcome(state) + assert resolution == expected_resolution + + # The invariant tying the two together: a "usable key" origin iff resolve + # yields a non-empty key; "none" iff resolve raises or yields "". + if origin in ("client", "server"): + assert resolution[0] == "key" and resolution[1] + else: + assert resolution == ("raised", None) or resolution == ("key", "") diff --git a/tests/test_telemetry.py b/tests/test_telemetry.py index 4a581d13..f2b31e86 100644 --- a/tests/test_telemetry.py +++ b/tests/test_telemetry.py @@ -1,9 +1,10 @@ # SPDX-License-Identifier: LicenseRef-Blockscout -from unittest.mock import ANY, AsyncMock, patch +from unittest.mock import ANY, AsyncMock, MagicMock, patch import pytest +from pro_api_key_helpers import ctx_with_header -from blockscout_mcp_server import analytics, telemetry +from blockscout_mcp_server import analytics, pro_api_key_context, telemetry from blockscout_mcp_server.config import config from blockscout_mcp_server.constants import ( COMMUNITY_TELEMETRY_ENDPOINT, @@ -11,6 +12,124 @@ RESOURCE_READ_EVENT, ) +# --------------------------------------------------------------------------- +# resolve_auth_signals — shared derivation entry point + all-disabled short-circuit +# --------------------------------------------------------------------------- + + +def test_resolve_auth_signals_short_circuits_when_all_telemetry_disabled(monkeypatch): + """With HTTP mode off AND community telemetry disabled, derivation is skipped entirely. + + No sink can consume the signals in that state, so the helper must return + (None, None) without extracting ctx or hashing a key. + """ + monkeypatch.setattr(config, "disable_community_telemetry", True, raising=False) + analytics.set_http_mode(False) + compute_spy = MagicMock() + monkeypatch.setattr("blockscout_mcp_server.telemetry.compute_auth_signals", compute_spy) + + assert telemetry.resolve_auth_signals(object()) == (None, None) + compute_spy.assert_not_called() + + +def test_resolve_auth_signals_derives_when_community_enabled(monkeypatch): + """Community telemetry enabled (HTTP mode off) still needs the signals -> derivation runs. + + The community sink consumes the fingerprint, so the helper delegates to + compute_auth_signals and returns its pair unchanged. + """ + monkeypatch.setattr(config, "disable_community_telemetry", False, raising=False) + analytics.set_http_mode(False) + compute_spy = MagicMock(return_value=("server", "d" * 64)) + monkeypatch.setattr("blockscout_mcp_server.telemetry.compute_auth_signals", compute_spy) + + ctx = object() + assert telemetry.resolve_auth_signals(ctx) == ("server", "d" * 64) + compute_spy.assert_called_once_with(ctx) + + +def test_resolve_auth_signals_derives_in_http_mode_even_if_community_disabled(monkeypatch): + """HTTP mode on feeds the analytics sink, so derivation runs even when community is disabled. + + The short-circuit is a superset guard keyed only on "no sink at all"; with HTTP + mode on it never fires, so the helper delegates to compute_auth_signals. + """ + monkeypatch.setattr(config, "disable_community_telemetry", True, raising=False) + analytics.set_http_mode(True) + try: + compute_spy = MagicMock(return_value=("client", "e" * 64)) + monkeypatch.setattr("blockscout_mcp_server.telemetry.compute_auth_signals", compute_spy) + + ctx = object() + assert telemetry.resolve_auth_signals(ctx) == ("client", "e" * 64) + compute_spy.assert_called_once_with(ctx) + finally: + analytics.set_http_mode(False) + + +def test_resolve_auth_signals_returns_server_fingerprint_in_http_mode(monkeypatch): + """End-to-end: absent client key + server key + HTTP on + community off -> ("server", ). + + Uses the real compute_auth_signals (no spy). The server-key SHA-256 is no longer + gated away in this exact config — it is memoized once per process — so the helper + returns the full ("server", fingerprint) pair rather than ("server", None). + """ + monkeypatch.setattr(config, "disable_community_telemetry", True, raising=False) + monkeypatch.setattr(config, "pro_api_key_header", "Blockscout-MCP-Pro-Api-Key", raising=False) + monkeypatch.setattr(config, "pro_api_key", "server-key", raising=False) + analytics.set_http_mode(True) + try: + ctx = ctx_with_header("Blockscout-MCP-Pro-Api-Key", "") + expected = pro_api_key_context._fingerprint_pro_api_key("server-key") + assert telemetry.resolve_auth_signals(ctx) == ("server", expected) + finally: + analytics.set_http_mode(False) + + +def test_resolve_auth_signals_never_raises(monkeypatch): + """A hypothetical failure in compute_auth_signals degrades to (None, None), never propagates.""" + monkeypatch.setattr(config, "disable_community_telemetry", False, raising=False) + analytics.set_http_mode(False) + monkeypatch.setattr( + "blockscout_mcp_server.telemetry.compute_auth_signals", + MagicMock(side_effect=RuntimeError("boom")), + ) + + assert telemetry.resolve_auth_signals(object()) == (None, None) + + +def test_resolve_auth_signals_never_raises_when_gate_raises(monkeypatch): + """A failure in the gate itself degrades to (None, None): the gate is inside the try. + + Guards the contract that no observability concern — not even evaluating the + telemetry-active precondition — can propagate into the tool body. + """ + monkeypatch.setattr( + "blockscout_mcp_server.telemetry.is_any_telemetry_active", + MagicMock(side_effect=RuntimeError("boom")), + ) + + assert telemetry.resolve_auth_signals(object()) == (None, None) + + +@pytest.mark.parametrize( + ("http_mode", "community_disabled", "expected"), + [ + (True, True, True), # analytics sink live -> active + (True, False, True), # both sinks live -> active + (False, False, True), # community sink live -> active + (False, True, False), # provably off -> inactive + ], +) +def test_is_any_telemetry_active_truth_table(monkeypatch, http_mode, community_disabled, expected): + """Reports inactive only when telemetry is provably off (not HTTP mode AND community disabled).""" + monkeypatch.setattr(config, "disable_community_telemetry", community_disabled, raising=False) + analytics.set_http_mode(http_mode) + try: + assert telemetry.is_any_telemetry_active() is expected + finally: + analytics.set_http_mode(False) + @pytest.mark.asyncio async def test_send_community_usage_report_sent(monkeypatch): @@ -30,6 +149,8 @@ async def test_send_community_usage_report_sent(monkeypatch): "client_name": "client", "client_version": "1.0", "protocol_version": "1.1", + "auth_origin": None, + "api_key_fingerprint": None, }, headers=ANY, timeout=2.0, @@ -70,6 +191,106 @@ async def test_send_community_usage_report_network_error(monkeypatch): mock_client.post.assert_awaited_once() +@pytest.mark.asyncio +async def test_send_community_usage_report_includes_auth_origin_and_fingerprint(monkeypatch): + """A call passing auth_origin='client' and a fingerprint string includes both verbatim.""" + monkeypatch.setattr(config, "disable_community_telemetry", False, raising=False) + monkeypatch.setattr(config, "mixpanel_token", "", raising=False) + fingerprint = "a" * 64 + mock_client = AsyncMock() + mock_ctx_mgr = AsyncMock() + mock_ctx_mgr.__aenter__.return_value = mock_client + with patch("httpx.AsyncClient", return_value=mock_ctx_mgr): + await telemetry.send_community_usage_report( + "tool", + {"a": 1}, + "client", + "1.0", + "1.1", + auth_origin="client", + api_key_fingerprint=fingerprint, + ) + url = f"{COMMUNITY_TELEMETRY_URL}{COMMUNITY_TELEMETRY_ENDPOINT}" + mock_client.post.assert_awaited_once_with( + url, + json={ + "tool_name": "tool", + "tool_args": {"a": 1}, + "client_name": "client", + "client_version": "1.0", + "protocol_version": "1.1", + "auth_origin": "client", + "api_key_fingerprint": fingerprint, + }, + headers=ANY, + timeout=2.0, + ) + + +@pytest.mark.asyncio +async def test_send_community_usage_report_none_origin_serializes_null_fingerprint(monkeypatch): + """A call passing auth_origin='none' and api_key_fingerprint=None serializes the fingerprint as JSON null.""" + monkeypatch.setattr(config, "disable_community_telemetry", False, raising=False) + monkeypatch.setattr(config, "mixpanel_token", "", raising=False) + mock_client = AsyncMock() + mock_ctx_mgr = AsyncMock() + mock_ctx_mgr.__aenter__.return_value = mock_client + with patch("httpx.AsyncClient", return_value=mock_ctx_mgr): + await telemetry.send_community_usage_report( + "tool", + {"a": 1}, + "client", + "1.0", + "1.1", + auth_origin="none", + api_key_fingerprint=None, + ) + url = f"{COMMUNITY_TELEMETRY_URL}{COMMUNITY_TELEMETRY_ENDPOINT}" + _, kwargs = mock_client.post.call_args + assert "api_key_fingerprint" in kwargs["json"] + assert kwargs["json"]["api_key_fingerprint"] is None + mock_client.post.assert_awaited_once_with( + url, + json={ + "tool_name": "tool", + "tool_args": {"a": 1}, + "client_name": "client", + "client_version": "1.0", + "protocol_version": "1.1", + "auth_origin": "none", + "api_key_fingerprint": None, + }, + headers=ANY, + timeout=2.0, + ) + + +@pytest.mark.asyncio +async def test_send_community_usage_report_raw_key_never_in_payload(monkeypatch): + """A representative raw key value never appears anywhere in the posted payload.""" + monkeypatch.setattr(config, "disable_community_telemetry", False, raising=False) + monkeypatch.setattr(config, "mixpanel_token", "", raising=False) + raw_key = "super-secret-raw-pro-api-key-value" + fingerprint = "b" * 64 + mock_client = AsyncMock() + mock_ctx_mgr = AsyncMock() + mock_ctx_mgr.__aenter__.return_value = mock_client + with patch("httpx.AsyncClient", return_value=mock_ctx_mgr): + await telemetry.send_community_usage_report( + "tool", + {"a": 1}, + "client", + "1.0", + "1.1", + auth_origin="client", + api_key_fingerprint=fingerprint, + ) + _, kwargs = mock_client.post.call_args + posted_payload = kwargs["json"] + assert raw_key not in str(posted_payload) + assert posted_payload["api_key_fingerprint"] == fingerprint + + # --------------------------------------------------------------------------- # send_community_resource_report tests # --------------------------------------------------------------------------- @@ -95,6 +316,44 @@ async def test_send_community_resource_report_sent(monkeypatch): "client_name": "client", "client_version": "1.0", "protocol_version": "1.1", + "auth_origin": None, + "api_key_fingerprint": None, + }, + headers=ANY, + timeout=2.0, + ) + + +@pytest.mark.asyncio +async def test_send_community_resource_report_forwards_auth_origin_and_fingerprint(monkeypatch): + """auth_origin and api_key_fingerprint passed in are forwarded verbatim to the delegated call.""" + monkeypatch.setattr(config, "disable_community_telemetry", False, raising=False) + monkeypatch.setattr(config, "mixpanel_token", "", raising=False) + uri = "blockscout-mcp://skill/SKILL.md" + fingerprint = "c" * 64 + mock_client = AsyncMock() + mock_ctx_mgr = AsyncMock() + mock_ctx_mgr.__aenter__.return_value = mock_client + with patch("httpx.AsyncClient", return_value=mock_ctx_mgr): + await telemetry.send_community_resource_report( + uri, + "client", + "1.0", + "1.1", + auth_origin="server", + api_key_fingerprint=fingerprint, + ) + url = f"{COMMUNITY_TELEMETRY_URL}{COMMUNITY_TELEMETRY_ENDPOINT}" + mock_client.post.assert_awaited_once_with( + url, + json={ + "tool_name": RESOURCE_READ_EVENT, + "tool_args": {"uri": uri}, + "client_name": "client", + "client_version": "1.0", + "protocol_version": "1.1", + "auth_origin": "server", + "api_key_fingerprint": fingerprint, }, headers=ANY, timeout=2.0, diff --git a/tests/test_tool_usage_report.py b/tests/test_tool_usage_report.py new file mode 100644 index 00000000..080609e3 --- /dev/null +++ b/tests/test_tool_usage_report.py @@ -0,0 +1,171 @@ +# SPDX-License-Identifier: LicenseRef-Blockscout +"""Tests for the `ToolUsageReport` Pydantic model's optional auth-signal fields.""" + +import logging + +import pytest + +from blockscout_mcp_server.models import ToolUsageReport + +_MODELS_LOGGER = "blockscout_mcp_server.models" + +VALID_FINGERPRINT = "a" * 64 + + +def _base_payload(**overrides): + payload = { + "tool_name": "get_block_info", + "tool_args": {"chain_id": "1"}, + "client_name": "test-client", + "client_version": "1.0.0", + "protocol_version": "2024-11-05", + } + payload.update(overrides) + return payload + + +def test_legacy_payload_defaults_new_fields_to_none(): + """A payload with only the five original fields validates, and both new fields default to None.""" + report = ToolUsageReport(**_base_payload()) + + assert report.auth_origin is None + assert report.api_key_fingerprint is None + + +@pytest.mark.parametrize("origin", ["client", "server", "none"]) +def test_auth_origin_round_trips_each_valid_value(origin): + """A payload supplying auth_origin as each of client/server/none validates and round-trips it.""" + report = ToolUsageReport(**_base_payload(auth_origin=origin)) + + assert report.auth_origin == origin + + +@pytest.mark.parametrize( + "unrecognized", + ["bogus", "Client", "SERVER", "unknown", 123, ["client"], {"origin": "client"}, 4.5], +) +def test_auth_origin_tolerates_unrecognized_value(unrecognized): + """An unrecognized auth_origin is coerced to None rather than rejected. + + Covers out-of-enum ("bogus"), wrong case ("Client"/"SERVER"), the "unknown" Mixpanel sentinel + (never a valid wire value), and non-string junk. auth_origin is consumed, but reports arrive + fire-and-forget from independently-versioned community instances, so a value this receiver does + not recognize must not drop the whole otherwise-valid report; None is rendered as the "unknown" + bucket downstream. The `mode="before"` validator guards with `isinstance(value, str)`, so + non-string input is tolerated too. + """ + report = ToolUsageReport(**_base_payload(auth_origin=unrecognized)) + + assert report.auth_origin is None + + +@pytest.mark.parametrize("value", ["proxy", "a" * 64]) +def test_unrecognized_string_auth_origin_logs_type_and_length_never_value(caplog, value): + """An unrecognized string auth_origin is logged by type and length only — never the value. + + A short version-skew member (e.g. a future "proxy") and long, secret-shaped junk (a 64-hex + fingerprint or raw key a buggy reporter swapped in) follow the identical path: the debug line + records that a coercion happened plus the value's type/length for version-skew visibility, but + the value itself never reaches the log. Mirrors the no-value-in-logs posture of the + api_key_fingerprint validator. + """ + with caplog.at_level(logging.DEBUG, logger=_MODELS_LOGGER): + ToolUsageReport(**_base_payload(auth_origin=value)) + + assert value not in caplog.text + assert "type=str" in caplog.text + assert f"len={len(value)}" in caplog.text + + +def test_non_string_auth_origin_logs_type_without_length(caplog): + """A non-string unrecognized auth_origin logs its type with ``len=n/a`` and never raises. + + The ``len(...)`` diagnostic is guarded by ``isinstance(value, str)``, so junk types (here a + list, which does have ``len``) still report ``n/a`` rather than a misleading element count. + """ + with caplog.at_level(logging.DEBUG, logger=_MODELS_LOGGER): + ToolUsageReport(**_base_payload(auth_origin=["client"])) + + assert "type=list" in caplog.text + assert "len=n/a" in caplog.text + + +def test_api_key_fingerprint_round_trips_valid_value(): + """A payload supplying a valid 64-char lowercase hex api_key_fingerprint validates and round-trips.""" + report = ToolUsageReport(**_base_payload(api_key_fingerprint=VALID_FINGERPRINT)) + + assert report.api_key_fingerprint == VALID_FINGERPRINT + + +def test_explicit_none_auth_origin_string_with_null_fingerprint_round_trips(): + """Explicitly setting auth_origin="none" and api_key_fingerprint=None round-trips None. + + This is the no-key wire shape an updated reporter actually sends (Phase 4 includes the key + unconditionally), distinct from omitting the fields. It guards against the `pattern` + constraint accidentally rejecting an explicit `null`. + """ + report = ToolUsageReport(**_base_payload(auth_origin="none", api_key_fingerprint=None)) + + assert report.auth_origin == "none" + assert report.api_key_fingerprint is None + + +def test_explicit_null_auth_origin_with_null_fingerprint_round_trips(): + """Explicitly setting auth_origin=None (Python None / JSON null) round-trips both fields as None. + + This pins the field's typing to `AuthOrigin | None`. A bare `Literal[...]` that merely + defaults to None would accept an omitted field but reject an explicit null; only this test + catches that, since the omitted and string-"none" cases pass either way. + """ + report = ToolUsageReport(**_base_payload(auth_origin=None, api_key_fingerprint=None)) + + assert report.auth_origin is None + assert report.api_key_fingerprint is None + + +def test_api_key_fingerprint_tolerates_too_short_value(): + """A malformed (too short) api_key_fingerprint is tolerated: coerced to None, not rejected. + + The fingerprint is a not-yet-consumed forward-compat field, so a malformed value must not + drop the otherwise-valid report. + """ + report = ToolUsageReport(**_base_payload(api_key_fingerprint="abc")) + + assert report.api_key_fingerprint is None + + +def test_api_key_fingerprint_tolerates_non_hex_value(): + """A 64-character but non-hex api_key_fingerprint is coerced to None rather than rejected.""" + report = ToolUsageReport(**_base_payload(api_key_fingerprint="z" * 64)) + + assert report.api_key_fingerprint is None + + +def test_api_key_fingerprint_tolerates_uppercase_hex_value(): + """A 64-character uppercase-hex api_key_fingerprint is coerced to None rather than rejected. + + Only lowercase hex is valid (matching the lowercase output of + `hashlib.sha256(...).hexdigest()`), so uppercase is treated as malformed and tolerated. + """ + report = ToolUsageReport(**_base_payload(api_key_fingerprint="A" * 64)) + + assert report.api_key_fingerprint is None + + +def test_api_key_fingerprint_tolerates_over_length_value(): + """An over-length api_key_fingerprint is coerced to None rather than rejected.""" + report = ToolUsageReport(**_base_payload(api_key_fingerprint="a" * 65)) + + assert report.api_key_fingerprint is None + + +@pytest.mark.parametrize("junk", [123, ["a" * 64], {"fingerprint": "a" * 64}, 45.6, True]) +def test_api_key_fingerprint_tolerates_non_string_junk(junk): + """A non-string junk api_key_fingerprint (int/list/dict/float/bool) is coerced to None. + + The `mode="before"` validator guards with `isinstance(value, str)` before the regex, so + non-string input never reaches the pattern and is tolerated as None rather than raising. + """ + report = ToolUsageReport(**_base_payload(api_key_fingerprint=junk)) + + assert report.api_key_fingerprint is None diff --git a/tests/tools/test_decorators.py b/tests/tools/test_decorators.py index 26848f2f..d7f5bab1 100644 --- a/tests/tools/test_decorators.py +++ b/tests/tools/test_decorators.py @@ -8,6 +8,7 @@ import pytest from mcp.server.fastmcp import Context from mcp.types import RequestParams +from pro_api_key_helpers import ctx_with_header from starlette.datastructures import Headers from blockscout_mcp_server.api.dependencies import MockCtx @@ -17,7 +18,12 @@ UNKNOWN_PROTOCOL_VERSION, ) from blockscout_mcp_server.config import config as server_config -from blockscout_mcp_server.pro_api_key_context import pro_api_key_scope, resolve_pro_api_key +from blockscout_mcp_server.pro_api_key_context import ( + _fingerprint_pro_api_key, + pro_api_key_scope, + resolve_pro_api_key, +) +from blockscout_mcp_server.telemetry import resolve_auth_signals from blockscout_mcp_server.tools.decorators import log_tool_invocation @@ -28,11 +34,12 @@ async def test_decorator_calls_analytics(monkeypatch, caplog: pytest.LogCaptureF calls = {} - def fake_track(ctx, name, args, client_meta=None): # type: ignore[no-untyped-def] + def fake_track(ctx, name, args, client_meta=None, auth_origin=None): # type: ignore[no-untyped-def] calls["ctx"] = ctx calls["name"] = name calls["args"] = args calls["client_meta"] = client_meta + calls["auth_origin"] = auth_origin monkeypatch.setattr("blockscout_mcp_server.tools.decorators.analytics.track_tool_invocation", fake_track) @@ -40,6 +47,19 @@ def fake_track(ctx, name, args, client_meta=None): # type: ignore[no-untyped-de async def dummy_tool(a: int, ctx: Context) -> int: return a + # Pin community telemetry on so resolve_auth_signals derives instead of short-circuiting to + # (None, None): with community disabled (e.g. BLOCKSCOUT_DISABLE_COMMUNITY_TELEMETRY set in the + # env or a local .env) and HTTP mode off, no sink would consume the signals and auth_origin + # would be None regardless of the header — making the assertion below untestable. + monkeypatch.setattr(server_config, "disable_community_telemetry", False, raising=False) + # A valid client PRO API key header makes the derived origin deterministic ("client"). + # Pinning the exact value is what gives the assertion teeth: `in ("client","server","none")` + # is vacuous because every AuthOrigin member trivially satisfies it, so it would pass even + # if the decorator threaded a constant or the wrong field. + monkeypatch.setattr(server_config, "pro_api_key", "", raising=False) + mock_ctx.session = None + mock_ctx.request_context = ctx_with_header(server_config.pro_api_key_header, "client-key-123").request_context + # Act await dummy_tool(7, ctx=mock_ctx) @@ -48,6 +68,10 @@ async def dummy_tool(a: int, ctx: Context) -> int: assert calls["args"] == {"a": 7} assert calls["ctx"] is mock_ctx assert "client_meta" in calls + # The decorator derives the auth signals from ctx and threads the origin into + # track_tool_invocation. With a valid client-key header present, that origin must be + # exactly "client" — proving a real ctx-derived value was threaded, not a constant. + assert calls["auth_origin"] == "client" @pytest.mark.asyncio @@ -125,7 +149,12 @@ async def dummy_tool(a: int, ctx: Context) -> int: "blockscout_mcp_server.tools.decorators.telemetry.send_community_usage_report", new_callable=AsyncMock, ) -async def test_decorator_reports_telemetry(mock_report, mock_ctx: Context) -> None: +async def test_decorator_reports_telemetry(mock_report, monkeypatch, mock_ctx: Context) -> None: + # Keep community telemetry enabled so resolve_auth_signals derives auth_origin instead of + # short-circuiting to (None, None) when BLOCKSCOUT_DISABLE_COMMUNITY_TELEMETRY is set ambiently. + monkeypatch.setattr(server_config, "disable_community_telemetry", False, raising=False) + monkeypatch.setattr(server_config, "pro_api_key", "", raising=False) + @log_tool_invocation async def dummy_tool(a: int, ctx: Context) -> int: return a @@ -140,9 +169,103 @@ async def dummy_tool(a: int, ctx: Context) -> int: UNDEFINED_CLIENT_NAME, UNDEFINED_CLIENT_VERSION, UNKNOWN_PROTOCOL_VERSION, + auth_origin="none", + api_key_fingerprint=None, ) +@pytest.mark.asyncio +@patch( + "blockscout_mcp_server.tools.decorators.telemetry.send_community_usage_report", + new_callable=AsyncMock, +) +async def test_decorator_reports_telemetry_with_client_key(mock_report, monkeypatch, mock_ctx: Context) -> None: + """A client-supplied PRO API key header is forwarded as a non-reversible fingerprint.""" + # Keep community telemetry enabled so resolve_auth_signals derives auth_origin instead of + # short-circuiting to (None, None) when BLOCKSCOUT_DISABLE_COMMUNITY_TELEMETRY is set ambiently. + monkeypatch.setattr(server_config, "disable_community_telemetry", False, raising=False) + monkeypatch.setattr(server_config, "pro_api_key", "", raising=False) + raw_key = "super-secret-client-key" + + @log_tool_invocation + async def dummy_tool(a: int, ctx: Context) -> int: + return a + + mock_ctx.session = None + mock_ctx.request_context = ctx_with_header(server_config.pro_api_key_header, raw_key).request_context + await dummy_tool(5, ctx=mock_ctx) + await asyncio.sleep(0) + + mock_report.assert_awaited_once() + call_kwargs = mock_report.await_args.kwargs + assert call_kwargs["auth_origin"] == "client" + assert call_kwargs["api_key_fingerprint"] is not None + assert call_kwargs["api_key_fingerprint"] != raw_key + + +@pytest.mark.asyncio +@patch( + "blockscout_mcp_server.tools.decorators.telemetry.send_community_usage_report", + new_callable=AsyncMock, +) +async def test_decorator_derives_auth_signals_once_and_threads_to_both_sinks( + mock_report, monkeypatch, mock_ctx: Context +) -> None: + """The (auth_origin, fingerprint) pair is derived a single time per invocation and the + identical values flow to both the Mixpanel sink and the community report — with no second + ctx extraction / SHA-256 on the hot path (regression guard for the de-dup fix). + + This drives the *real* analytics sink (only the Mixpanel client is mocked) and spies on the + real derivation helper rather than mocking it away. That is what makes the guard meaningful: + the origin is derived exactly once, by the decorator, and the identical value is observed in + the Mixpanel property bag — proving it was threaded through rather than recomputed by the sink. + Mocking the derivation away (the previous approach) made the "derived once" claim structurally + true by construction instead of observing it. + """ + # Spy on the single derivation point — the shared telemetry.resolve_auth_signals helper the + # decorator delegates to — while keeping the real implementation. + signals_spy = MagicMock(side_effect=resolve_auth_signals) + monkeypatch.setattr("blockscout_mcp_server.telemetry.resolve_auth_signals", signals_spy) + + # Drive the real analytics sink with a mocked Mixpanel client so the property bag is + # genuinely exercised rather than structurally bypassed. + mock_mp = MagicMock() + monkeypatch.setattr("blockscout_mcp_server.analytics._is_http_mode_enabled", True, raising=False) + monkeypatch.setattr("blockscout_mcp_server.analytics._get_mixpanel_client", lambda: mock_mp) + + # Deterministic valid client-key header -> origin "client", fingerprint = client-key hash. + monkeypatch.setattr(server_config, "pro_api_key", "", raising=False) + raw_key = "client-key-123" + mock_ctx.session = None + mock_ctx.request_context = ctx_with_header(server_config.pro_api_key_header, raw_key).request_context + mock_ctx.request_context.request.client = SimpleNamespace(host="127.0.0.1") + + @log_tool_invocation + async def dummy_tool(a: int, ctx: Context) -> int: + return a + + await dummy_tool(5, ctx=mock_ctx) + await asyncio.sleep(0) + + # Derived exactly once by the decorator... + signals_spy.assert_called_once() + + # ...and the same origin reached the Mixpanel property bag (3rd positional arg of mp.track), + # confirming the sink reused the threaded value instead of re-deriving it. + mock_mp.track.assert_called_once() + props = mock_mp.track.call_args.args[2] + assert props["auth_origin"] == "client" + # The fingerprint is never emitted to Mixpanel (privacy invariant). + assert "api_key_fingerprint" not in props + + # ...and the identical pair reached the community report. + expected_fingerprint = _fingerprint_pro_api_key(raw_key) + mock_report.assert_awaited_once() + call_kwargs = mock_report.await_args.kwargs + assert call_kwargs["auth_origin"] == "client" + assert call_kwargs["api_key_fingerprint"] == expected_fingerprint + + @pytest.mark.asyncio async def test_log_tool_invocation_logs_meta_fields(caplog: pytest.LogCaptureFixture, mock_ctx: Context) -> None: """Verify that MCP meta fields including OpenAI fields are logged in Tool invoked message."""