From 3d8aca3de5237d78b4c456af2ce1a127a7eeef5e Mon Sep 17 00:00:00 2001 From: avinahradau Date: Thu, 18 Jun 2026 16:32:38 +0100 Subject: [PATCH 1/3] fix(oauth): match backend access-token prefix for passthrough detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit opik-backend mints MCP OAuth access tokens with the prefix `opik_mcp_at_` (McpOAuthTokenUtils.ACCESS_PREFIX), but opik-mcp's OAuth-passthrough detection checked for `opik_at_`. The two never matched, so every real OAuth bearer was misclassified as a non-OAuth credential in resolve_opik_config: it took the API-key branch and forwarded a stale Comet-Workspace header (settings.comet_workspace, else "default"). opik-backend's AuthFilter — which recognizes the token correctly — then rejected the request with 403 "workspace does not match access token", because the token row is bound to the consented workspace and is never "default". Result: hosted OAuth MCP could not access any real workspace. Consolidate the prefix into a single OAUTH_ACCESS_TOKEN_PREFIX constant in auth_context and use it at both detection sites (classify_bearer and resolve_opik_config) so they can't drift again. Update comments, README, and tests to the correct prefix. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 2 +- src/opik_mcp/analytics/client.py | 2 +- src/opik_mcp/auth_context.py | 22 ++++++++++++++-------- src/opik_mcp/opik_client.py | 14 ++++++++++---- src/opik_mcp/server.py | 2 +- tests/test_analytics_auth_rejected.py | 10 +++++----- tests/test_analytics_client_build_event.py | 4 ++-- tests/test_analytics_privacy.py | 4 ++-- tests/test_http_auth.py | 2 +- tests/test_oauth_passthrough_mode.py | 6 +++--- tests/test_opik_client.py | 10 +++++----- 11 files changed, 45 insertions(+), 33 deletions(-) diff --git a/README.md b/README.md index 7a7388d..5b95f7d 100644 --- a/README.md +++ b/README.md @@ -326,7 +326,7 @@ Every setting is an environment variable. Required ones in **bold**. #### Choosing a transport opik-mcp performs **no local credential validation** on HTTP transport: any -well-formed `Authorization: Bearer …` (an Opik API key or an `opik_at_…` +well-formed `Authorization: Bearer …` (an Opik API key or an `opik_mcp_at_…` OAuth access token) is forwarded verbatim to opik-backend, which is the single point of auth enforcement. Pick the transport by deployment shape: diff --git a/src/opik_mcp/analytics/client.py b/src/opik_mcp/analytics/client.py index 0d96a14..d822ae1 100644 --- a/src/opik_mcp/analytics/client.py +++ b/src/opik_mcp/analytics/client.py @@ -260,7 +260,7 @@ def _per_request_props(self) -> dict[str, str]: ``auth_mode`` is ALWAYS set so stdio / no-auth events are not a dark cohort. PRIVACY: the raw bearer token never enters the result — only its - sha256 digest, and only for ``opik_at_`` OAuth tokens. ``request_workspace`` + sha256 digest, and only for ``opik_mcp_at_`` OAuth tokens. ``request_workspace`` mirrors the existing plaintext ``workspace`` posture (workspace names are used as ``user_id`` in ``resolve_anonymous_id``). """ diff --git a/src/opik_mcp/auth_context.py b/src/opik_mcp/auth_context.py index 4bfec29..82b2ccb 100644 --- a/src/opik_mcp/auth_context.py +++ b/src/opik_mcp/auth_context.py @@ -1,7 +1,7 @@ """Per-request inbound-auth propagation for OAuth-passthrough mode. When opik-mcp runs over HTTP transport with OAuth, the MCP host attaches -`Authorization: Bearer opik_at_…` per RFC 6750 and opik-mcp's job is to +`Authorization: Bearer opik_mcp_at_…` per RFC 6750 and opik-mcp's job is to forward that bearer onward to opik-backend's data API verbatim. Permission enforcement lives at the data API endpoint via `@RequiredPermissions` annotations; opik-mcp performs no local validation and makes no separate @@ -20,7 +20,13 @@ from contextvars import ContextVar -# Full inbound ``Authorization`` header value (e.g. ``"Bearer opik_at_…"``), +# Access-token prefix minted by opik-backend (McpOAuthTokenUtils.ACCESS_PREFIX). +# OAuth-passthrough detection MUST match the issuer: a mismatch makes a real +# OAuth bearer fall through to the API-key path, which then forwards a stale +# Comet-Workspace header that opik-backend rejects with 403. +OAUTH_ACCESS_TOKEN_PREFIX = "opik_mcp_at_" + +# Full inbound ``Authorization`` header value (e.g. ``"Bearer opik_mcp_at_…"``), # forwarded verbatim on outbound calls to opik-backend's data API. ``None`` # means "no inbound bearer; fall back to settings.opik_api_key". inbound_authorization: ContextVar[str | None] = ContextVar("inbound_authorization", default=None) @@ -36,20 +42,20 @@ def classify_bearer(auth_header: str) -> tuple[str, str]: """Classify a non-empty inbound ``Authorization`` header for BI analytics. Returns ``(auth_mode, oauth_token)``: - - ``("oauth", "")`` for an OAuth bearer — the token is returned + - ``("oauth", "")`` for an OAuth bearer — the token is returned ONLY so the caller can hash it; it is never stored or emitted raw. - ``("api_key", "")`` for any other forwarded credential (the token is NOT returned — api-key-shaped credentials are not hashed here). Mirrors ``opik_client.resolve_opik_config``'s OAuth detection - (``partition(" ")`` + ``lstrip`` + ``opik_at_`` prefix) so BI's ``auth_mode`` - / ``token_sha256`` agree with the credential actually forwarded outbound. - Single source of truth shared by ``analytics.client._build_event`` and - ``server.AuthRejectionMiddleware`` so the two cannot drift. + (``partition(" ")`` + ``lstrip`` + ``OAUTH_ACCESS_TOKEN_PREFIX``) so BI's + ``auth_mode`` / ``token_sha256`` agree with the credential actually forwarded + outbound. Single source of truth shared by ``analytics.client._build_event`` + and ``server.AuthRejectionMiddleware`` so the two cannot drift. """ scheme, _, token_raw = auth_header.partition(" ") token = token_raw.lstrip() - if scheme.lower() == "bearer" and token.startswith("opik_at_"): + if scheme.lower() == "bearer" and token.startswith(OAUTH_ACCESS_TOKEN_PREFIX): return "oauth", token return "api_key", "" diff --git a/src/opik_mcp/opik_client.py b/src/opik_mcp/opik_client.py index 19183c3..87e28c1 100644 --- a/src/opik_mcp/opik_client.py +++ b/src/opik_mcp/opik_client.py @@ -20,7 +20,11 @@ import httpx -from opik_mcp.auth_context import inbound_authorization, inbound_workspace +from opik_mcp.auth_context import ( + OAUTH_ACCESS_TOKEN_PREFIX, + inbound_authorization, + inbound_workspace, +) from opik_mcp.config import DEFAULT_WORKSPACE, MissingConfigError, Settings from opik_mcp.error_kinds import ErrorKind @@ -628,7 +632,7 @@ def resolve_opik_config(settings: Settings) -> tuple[str, str, str | None]: header (OAuth-passthrough mode), the middleware populates :mod:`opik_mcp.auth_context` ContextVars and we prefer those over the env-bound ``OPIK_API_KEY`` / ``COMET_WORKSPACE``. opik-backend's - ``AuthFilter`` accepts both shapes (API key and ``Bearer opik_at_…``) + ``AuthFilter`` accepts both shapes (API key and ``Bearer opik_mcp_at_…``) and enforces ``@RequiredPermissions`` per endpoint, so opik-mcp is a thin forwarder either way. """ @@ -641,12 +645,14 @@ def resolve_opik_config(settings: Settings) -> tuple[str, str, str | None]: ) # OAuth access tokens carry their workspace server-side (opik-backend # derives it from the token row). Identified by token prefix — mirrors - # the backend's McpOAuthTokens.isMcpOAuthToken — so an API key that + # the backend's McpOAuthTokenUtils.isMcpOAuthToken — so an API key that # merely contains the marker can't skip the workspace requirement. oauth_passthrough = False if inbound_auth is not None: scheme, _, token = inbound_auth.partition(" ") - oauth_passthrough = scheme.lower() == "bearer" and token.lstrip().startswith("opik_at_") + oauth_passthrough = scheme.lower() == "bearer" and token.lstrip().startswith( + OAUTH_ACCESS_TOKEN_PREFIX + ) if oauth_passthrough: # Workspace is derived from the token server-side; may be None here. workspace = inbound_ws diff --git a/src/opik_mcp/server.py b/src/opik_mcp/server.py index 29c32ed..dcde0b3 100644 --- a/src/opik_mcp/server.py +++ b/src/opik_mcp/server.py @@ -592,7 +592,7 @@ class BearerAuthMiddleware(BaseHTTPMiddleware): captured into a ContextVar and forwarded verbatim on the outbound call to opik-backend (see :mod:`opik_mcp.auth_context`). opik-backend's ``AuthFilter`` is the single point of auth enforcement — it validates the - bearer (API key or ``opik_at_…`` OAuth token) and enforces + bearer (API key or ``opik_mcp_at_…`` OAuth token) and enforces ``@RequiredPermissions`` on the data API endpoint. Deployments where the backend enforces auth are protected end-to-end; OSS installs without backend auth are as open via MCP as via their own REST API. diff --git a/tests/test_analytics_auth_rejected.py b/tests/test_analytics_auth_rejected.py index 3abc82c..8c034f7 100644 --- a/tests/test_analytics_auth_rejected.py +++ b/tests/test_analytics_auth_rejected.py @@ -17,7 +17,7 @@ from opik_mcp.config import Settings # Unique bearer token canary — must never appear raw in the emitted event. -RAW_TOKEN_CANARY = "opik_at_AUTHREJECT-CANARY-TOKEN-3f9a2b1c" +RAW_TOKEN_CANARY = "opik_mcp_at_AUTHREJECT-CANARY-TOKEN-3f9a2b1c" class _Recorder: @@ -94,19 +94,19 @@ def test_empty_bearer_token_reason(monkeypatch: pytest.MonkeyPatch) -> None: def test_421_is_host_rejected(monkeypatch: pytest.MonkeyPatch) -> None: recorder, mw = _make(monkeypatch, 421) - _drive(mw, path="/mcp", auth=b"Bearer opik_at_x") + _drive(mw, path="/mcp", auth=b"Bearer opik_mcp_at_x") assert _only(recorder)["rejection_reason"] == "host_rejected" def test_403_is_origin_rejected(monkeypatch: pytest.MonkeyPatch) -> None: recorder, mw = _make(monkeypatch, 403) - _drive(mw, path="/mcp", auth=b"Bearer opik_at_x") + _drive(mw, path="/mcp", auth=b"Bearer opik_mcp_at_x") assert _only(recorder)["rejection_reason"] == "origin_rejected" def test_success_emits_no_event(monkeypatch: pytest.MonkeyPatch) -> None: recorder, mw = _make(monkeypatch, 200) - _drive(mw, path="/mcp", auth=b"Bearer opik_at_x") + _drive(mw, path="/mcp", auth=b"Bearer opik_mcp_at_x") assert not [p for et, p in recorder.events if et == "opik_mcp_auth_rejected"] @@ -177,7 +177,7 @@ def test_auth_mode_reflects_rejected_oauth_bearer(monkeypatch: pytest.MonkeyPatc # auth_mode="oauth" — derived from the header, NOT the already-reset # ContextVar (which would yield the settings fallback). recorder, mw = _make(monkeypatch, 421) - _drive(mw, path="/mcp", auth=b"Bearer opik_at_valid-token") + _drive(mw, path="/mcp", auth=b"Bearer opik_mcp_at_valid-token") props = _only(recorder) assert props["rejection_reason"] == "host_rejected" assert props["auth_mode"] == "oauth" diff --git a/tests/test_analytics_client_build_event.py b/tests/test_analytics_client_build_event.py index b6c34bc..ea0f3e4 100644 --- a/tests/test_analytics_client_build_event.py +++ b/tests/test_analytics_client_build_event.py @@ -25,7 +25,7 @@ from opik_mcp.config import Settings # Canaries: unique, greppable values that must never appear raw in an event. -RAW_OAUTH_TOKEN = "opik_at_BEARER-CANARY-TOKEN-UNIQUE-7a3b2c1d" +RAW_OAUTH_TOKEN = "opik_mcp_at_BEARER-CANARY-TOKEN-UNIQUE-7a3b2c1d" RAW_WORKSPACE = "WORKSPACE-CANARY-NAME-MUST-NOT-LEAK-9f4e5a6b" @@ -80,7 +80,7 @@ def test_non_oauth_bearer_is_api_key_mode_without_token_hash(make_client: Any) - with _inbound(auth="Bearer some-non-oauth-static-key"): props = client._build_event("opik_mcp_tool_called", {})["event_properties"] assert props["auth_mode"] == "api_key" - assert "token_sha256" not in props # only opik_at_ bearers are hashed + assert "token_sha256" not in props # only opik_mcp_at_ bearers are hashed def test_request_workspace_plaintext_present(make_client: Any) -> None: diff --git a/tests/test_analytics_privacy.py b/tests/test_analytics_privacy.py index 23036e7..4c07e3d 100644 --- a/tests/test_analytics_privacy.py +++ b/tests/test_analytics_privacy.py @@ -50,7 +50,7 @@ "FORBIDDEN-CANARY-home-path-5e6f7a8b", # OAuth bearer token canary — auth_rejected derives its reason from the # header SHAPE only; the raw token must never reach the event. - "opik_at_FORBIDDEN-CANARY-oauth-token-2f8e1d3c", + "opik_mcp_at_FORBIDDEN-CANARY-oauth-token-2f8e1d3c", ] @@ -883,7 +883,7 @@ def hi() -> str: "type": "http", "path": "/mcp", "headers": [ - (b"authorization", b"Bearer opik_at_FORBIDDEN-CANARY-oauth-token-2f8e1d3c"), + (b"authorization", b"Bearer opik_mcp_at_FORBIDDEN-CANARY-oauth-token-2f8e1d3c"), ], } mw._emit_rejection(scope, 401) diff --git a/tests/test_http_auth.py b/tests/test_http_auth.py index bb09d55..5bd035b 100644 --- a/tests/test_http_auth.py +++ b/tests/test_http_auth.py @@ -48,7 +48,7 @@ async def test_any_bearer_initializes(http_client: httpx.AsyncClient) -> None: "/mcp", json=INITIALIZE, headers={ - "Authorization": "Bearer opik_at_anything", + "Authorization": "Bearer opik_mcp_at_anything", "Accept": "application/json, text/event-stream", }, ) diff --git a/tests/test_oauth_passthrough_mode.py b/tests/test_oauth_passthrough_mode.py index 77efcd4..300fa4a 100644 --- a/tests/test_oauth_passthrough_mode.py +++ b/tests/test_oauth_passthrough_mode.py @@ -51,7 +51,7 @@ async def test_passthrough_accepts_any_well_formed_bearer() -> None: opik-backend's AuthFilter validates the token; opik-mcp is a thin pipe. """ mw = _build_middleware() - request = _make_request({"authorization": "Bearer opik_at_abc123"}) + request = _make_request({"authorization": "Bearer opik_mcp_at_abc123"}) captured: dict[str, str | None] = {} @@ -64,7 +64,7 @@ async def call_next(_r: Request) -> Response: assert resp.status_code == 200 # Bearer captured exactly as inbound so outbound forwarding preserves it. - assert captured["auth"] == "Bearer opik_at_abc123" + assert captured["auth"] == "Bearer opik_mcp_at_abc123" # No workspace header on this request → ContextVar reads as None. assert captured["workspace"] is None # ContextVar is reset after the request returns — no leakage to the @@ -78,7 +78,7 @@ async def test_passthrough_captures_comet_workspace_header() -> None: mw = _build_middleware() request = _make_request( { - "authorization": "Bearer opik_at_abc", + "authorization": "Bearer opik_mcp_at_abc", "comet-workspace": "my-team", } ) diff --git a/tests/test_opik_client.py b/tests/test_opik_client.py index 3371a9c..6d7c21e 100644 --- a/tests/test_opik_client.py +++ b/tests/test_opik_client.py @@ -363,13 +363,13 @@ def test_resolve_opik_config_oauth_token_makes_workspace_optional() -> None: from opik_mcp.opik_client import resolve_opik_config s = Settings(opik_api_key=None, comet_workspace=None, opik_url="https://opik.example.com") - token = inbound_authorization.set("Bearer opik_at_abc123") + token = inbound_authorization.set("Bearer opik_mcp_at_abc123") try: _base, api_key, workspace = resolve_opik_config(s) finally: inbound_authorization.reset(token) - assert api_key == "Bearer opik_at_abc123" + assert api_key == "Bearer opik_mcp_at_abc123" assert workspace is None @@ -383,7 +383,7 @@ def test_resolve_opik_config_oauth_detection_is_prefix_not_substring() -> None: from opik_mcp.opik_client import resolve_opik_config s = Settings(opik_api_key=None, comet_workspace=None, opik_url="https://opik.example.com") - token = inbound_authorization.set("Bearer sk-xopik_at_y") + token = inbound_authorization.set("Bearer sk-xopik_mcp_at_y") try: _base, _api_key, workspace = resolve_opik_config(s) finally: @@ -395,11 +395,11 @@ def test_oauth_client_omits_workspace_header() -> None: from opik_mcp.opik_client import OpikClient client = OpikClient( - base_url="https://opik.example.com", api_key="Bearer opik_at_x", workspace=None + base_url="https://opik.example.com", api_key="Bearer opik_mcp_at_x", workspace=None ) headers = client._headers() assert "Comet-Workspace" not in headers - assert headers["Authorization"] == "Bearer opik_at_x" + assert headers["Authorization"] == "Bearer opik_mcp_at_x" # --- execute_experiment ------------------------------------------------------- # From 5a3a1af386efe769462c17d91581676ec28750b9 Mon Sep 17 00:00:00 2001 From: avinahradau Date: Thu, 18 Jun 2026 16:39:24 +0100 Subject: [PATCH 2/3] refactor(oauth): reference OAUTH_ACCESS_TOKEN_PREFIX in src docs instead of the raw literal MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Keep the raw "opik_mcp_at_" string in exactly one place — the constant definition — and have every other src/ comment and docstring refer to it by name. Single source of truth; no functional change. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/opik_mcp/analytics/client.py | 2 +- src/opik_mcp/auth_context.py | 11 ++++++----- src/opik_mcp/opik_client.py | 2 +- src/opik_mcp/server.py | 2 +- 4 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/opik_mcp/analytics/client.py b/src/opik_mcp/analytics/client.py index d822ae1..2e93e50 100644 --- a/src/opik_mcp/analytics/client.py +++ b/src/opik_mcp/analytics/client.py @@ -260,7 +260,7 @@ def _per_request_props(self) -> dict[str, str]: ``auth_mode`` is ALWAYS set so stdio / no-auth events are not a dark cohort. PRIVACY: the raw bearer token never enters the result — only its - sha256 digest, and only for ``opik_mcp_at_`` OAuth tokens. ``request_workspace`` + sha256 digest, and only for ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed OAuth tokens. ``request_workspace`` mirrors the existing plaintext ``workspace`` posture (workspace names are used as ``user_id`` in ``resolve_anonymous_id``). """ diff --git a/src/opik_mcp/auth_context.py b/src/opik_mcp/auth_context.py index 82b2ccb..b82db10 100644 --- a/src/opik_mcp/auth_context.py +++ b/src/opik_mcp/auth_context.py @@ -1,7 +1,8 @@ """Per-request inbound-auth propagation for OAuth-passthrough mode. When opik-mcp runs over HTTP transport with OAuth, the MCP host attaches -`Authorization: Bearer opik_mcp_at_…` per RFC 6750 and opik-mcp's job is to +`Authorization: Bearer …` (an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed token) +per RFC 6750 and opik-mcp's job is to forward that bearer onward to opik-backend's data API verbatim. Permission enforcement lives at the data API endpoint via `@RequiredPermissions` annotations; opik-mcp performs no local validation and makes no separate @@ -26,8 +27,8 @@ # Comet-Workspace header that opik-backend rejects with 403. OAUTH_ACCESS_TOKEN_PREFIX = "opik_mcp_at_" -# Full inbound ``Authorization`` header value (e.g. ``"Bearer opik_mcp_at_…"``), -# forwarded verbatim on outbound calls to opik-backend's data API. ``None`` +# Full inbound ``Authorization`` header value (an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed +# bearer), forwarded verbatim on outbound calls to opik-backend's data API. ``None`` # means "no inbound bearer; fall back to settings.opik_api_key". inbound_authorization: ContextVar[str | None] = ContextVar("inbound_authorization", default=None) @@ -42,8 +43,8 @@ def classify_bearer(auth_header: str) -> tuple[str, str]: """Classify a non-empty inbound ``Authorization`` header for BI analytics. Returns ``(auth_mode, oauth_token)``: - - ``("oauth", "")`` for an OAuth bearer — the token is returned - ONLY so the caller can hash it; it is never stored or emitted raw. + - ``("oauth", token)`` for an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed bearer — the + token is returned ONLY so the caller can hash it; never stored or emitted raw. - ``("api_key", "")`` for any other forwarded credential (the token is NOT returned — api-key-shaped credentials are not hashed here). diff --git a/src/opik_mcp/opik_client.py b/src/opik_mcp/opik_client.py index 87e28c1..d6de1a3 100644 --- a/src/opik_mcp/opik_client.py +++ b/src/opik_mcp/opik_client.py @@ -632,7 +632,7 @@ def resolve_opik_config(settings: Settings) -> tuple[str, str, str | None]: header (OAuth-passthrough mode), the middleware populates :mod:`opik_mcp.auth_context` ContextVars and we prefer those over the env-bound ``OPIK_API_KEY`` / ``COMET_WORKSPACE``. opik-backend's - ``AuthFilter`` accepts both shapes (API key and ``Bearer opik_mcp_at_…``) + ``AuthFilter`` accepts both shapes (API key and an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed ``Bearer``) and enforces ``@RequiredPermissions`` per endpoint, so opik-mcp is a thin forwarder either way. """ diff --git a/src/opik_mcp/server.py b/src/opik_mcp/server.py index dcde0b3..fc401b1 100644 --- a/src/opik_mcp/server.py +++ b/src/opik_mcp/server.py @@ -592,7 +592,7 @@ class BearerAuthMiddleware(BaseHTTPMiddleware): captured into a ContextVar and forwarded verbatim on the outbound call to opik-backend (see :mod:`opik_mcp.auth_context`). opik-backend's ``AuthFilter`` is the single point of auth enforcement — it validates the - bearer (API key or ``opik_mcp_at_…`` OAuth token) and enforces + bearer (API key or an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed OAuth token) and enforces ``@RequiredPermissions`` on the data API endpoint. Deployments where the backend enforces auth are protected end-to-end; OSS installs without backend auth are as open via MCP as via their own REST API. From 2ec08058974523f7046eb5fbab01f67aae53221a Mon Sep 17 00:00:00 2001 From: avinahradau Date: Thu, 18 Jun 2026 16:52:48 +0100 Subject: [PATCH 3/3] test(oauth): build token literals from OAUTH_ACCESS_TOKEN_PREFIX Import the constant in the auth/analytics tests and construct sample bearers/canaries as f-strings from it, so the suite tracks the prefix automatically and the raw "opik_mcp_at_" string survives only in the constant definition (README left as user-facing docs). No behavior change. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/opik_mcp/analytics/client.py | 6 +++--- src/opik_mcp/opik_client.py | 5 +++-- tests/test_analytics_auth_rejected.py | 11 ++++++----- tests/test_analytics_client_build_event.py | 10 +++++++--- tests/test_analytics_privacy.py | 6 ++++-- tests/test_http_auth.py | 4 +++- tests/test_oauth_passthrough_mode.py | 12 ++++++++---- tests/test_opik_client.py | 13 ++++++++----- 8 files changed, 42 insertions(+), 25 deletions(-) diff --git a/src/opik_mcp/analytics/client.py b/src/opik_mcp/analytics/client.py index 2e93e50..e2f961c 100644 --- a/src/opik_mcp/analytics/client.py +++ b/src/opik_mcp/analytics/client.py @@ -260,9 +260,9 @@ def _per_request_props(self) -> dict[str, str]: ``auth_mode`` is ALWAYS set so stdio / no-auth events are not a dark cohort. PRIVACY: the raw bearer token never enters the result — only its - sha256 digest, and only for ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed OAuth tokens. ``request_workspace`` - mirrors the existing plaintext ``workspace`` posture (workspace names are - used as ``user_id`` in ``resolve_anonymous_id``). + sha256 digest, and only for ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed OAuth + tokens. ``request_workspace`` mirrors the existing plaintext ``workspace`` + posture (workspace names are used as ``user_id`` in ``resolve_anonymous_id``). """ props: dict[str, str] = {} try: diff --git a/src/opik_mcp/opik_client.py b/src/opik_mcp/opik_client.py index d6de1a3..26726ff 100644 --- a/src/opik_mcp/opik_client.py +++ b/src/opik_mcp/opik_client.py @@ -632,8 +632,9 @@ def resolve_opik_config(settings: Settings) -> tuple[str, str, str | None]: header (OAuth-passthrough mode), the middleware populates :mod:`opik_mcp.auth_context` ContextVars and we prefer those over the env-bound ``OPIK_API_KEY`` / ``COMET_WORKSPACE``. opik-backend's - ``AuthFilter`` accepts both shapes (API key and an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed ``Bearer``) - and enforces ``@RequiredPermissions`` per endpoint, so opik-mcp is a + ``AuthFilter`` accepts both shapes (API key and an + ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed ``Bearer``) and enforces + ``@RequiredPermissions`` per endpoint, so opik-mcp is a thin forwarder either way. """ inbound_auth = inbound_authorization.get() diff --git a/tests/test_analytics_auth_rejected.py b/tests/test_analytics_auth_rejected.py index 8c034f7..ac31ff7 100644 --- a/tests/test_analytics_auth_rejected.py +++ b/tests/test_analytics_auth_rejected.py @@ -14,10 +14,11 @@ import pytest +from opik_mcp.auth_context import OAUTH_ACCESS_TOKEN_PREFIX from opik_mcp.config import Settings # Unique bearer token canary — must never appear raw in the emitted event. -RAW_TOKEN_CANARY = "opik_mcp_at_AUTHREJECT-CANARY-TOKEN-3f9a2b1c" +RAW_TOKEN_CANARY = f"{OAUTH_ACCESS_TOKEN_PREFIX}AUTHREJECT-CANARY-TOKEN-3f9a2b1c" class _Recorder: @@ -94,19 +95,19 @@ def test_empty_bearer_token_reason(monkeypatch: pytest.MonkeyPatch) -> None: def test_421_is_host_rejected(monkeypatch: pytest.MonkeyPatch) -> None: recorder, mw = _make(monkeypatch, 421) - _drive(mw, path="/mcp", auth=b"Bearer opik_mcp_at_x") + _drive(mw, path="/mcp", auth=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x".encode()) assert _only(recorder)["rejection_reason"] == "host_rejected" def test_403_is_origin_rejected(monkeypatch: pytest.MonkeyPatch) -> None: recorder, mw = _make(monkeypatch, 403) - _drive(mw, path="/mcp", auth=b"Bearer opik_mcp_at_x") + _drive(mw, path="/mcp", auth=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x".encode()) assert _only(recorder)["rejection_reason"] == "origin_rejected" def test_success_emits_no_event(monkeypatch: pytest.MonkeyPatch) -> None: recorder, mw = _make(monkeypatch, 200) - _drive(mw, path="/mcp", auth=b"Bearer opik_mcp_at_x") + _drive(mw, path="/mcp", auth=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x".encode()) assert not [p for et, p in recorder.events if et == "opik_mcp_auth_rejected"] @@ -177,7 +178,7 @@ def test_auth_mode_reflects_rejected_oauth_bearer(monkeypatch: pytest.MonkeyPatc # auth_mode="oauth" — derived from the header, NOT the already-reset # ContextVar (which would yield the settings fallback). recorder, mw = _make(monkeypatch, 421) - _drive(mw, path="/mcp", auth=b"Bearer opik_mcp_at_valid-token") + _drive(mw, path="/mcp", auth=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}valid-token".encode()) props = _only(recorder) assert props["rejection_reason"] == "host_rejected" assert props["auth_mode"] == "oauth" diff --git a/tests/test_analytics_client_build_event.py b/tests/test_analytics_client_build_event.py index ea0f3e4..0189da2 100644 --- a/tests/test_analytics_client_build_event.py +++ b/tests/test_analytics_client_build_event.py @@ -21,11 +21,15 @@ import pytest from opik_mcp.analytics.client import AnalyticsClient -from opik_mcp.auth_context import inbound_authorization, inbound_workspace +from opik_mcp.auth_context import ( + OAUTH_ACCESS_TOKEN_PREFIX, + inbound_authorization, + inbound_workspace, +) from opik_mcp.config import Settings # Canaries: unique, greppable values that must never appear raw in an event. -RAW_OAUTH_TOKEN = "opik_mcp_at_BEARER-CANARY-TOKEN-UNIQUE-7a3b2c1d" +RAW_OAUTH_TOKEN = f"{OAUTH_ACCESS_TOKEN_PREFIX}BEARER-CANARY-TOKEN-UNIQUE-7a3b2c1d" RAW_WORKSPACE = "WORKSPACE-CANARY-NAME-MUST-NOT-LEAK-9f4e5a6b" @@ -80,7 +84,7 @@ def test_non_oauth_bearer_is_api_key_mode_without_token_hash(make_client: Any) - with _inbound(auth="Bearer some-non-oauth-static-key"): props = client._build_event("opik_mcp_tool_called", {})["event_properties"] assert props["auth_mode"] == "api_key" - assert "token_sha256" not in props # only opik_mcp_at_ bearers are hashed + assert "token_sha256" not in props # only OAUTH_ACCESS_TOKEN_PREFIX-prefixed bearers are hashed def test_request_workspace_plaintext_present(make_client: Any) -> None: diff --git a/tests/test_analytics_privacy.py b/tests/test_analytics_privacy.py index 4c07e3d..e7153a6 100644 --- a/tests/test_analytics_privacy.py +++ b/tests/test_analytics_privacy.py @@ -23,6 +23,7 @@ import pytest +from opik_mcp.auth_context import OAUTH_ACCESS_TOKEN_PREFIX from opik_mcp.comet_client import PodDiscovery from opik_mcp.ollie_client import OnTick, SSEEvent @@ -50,7 +51,7 @@ "FORBIDDEN-CANARY-home-path-5e6f7a8b", # OAuth bearer token canary — auth_rejected derives its reason from the # header SHAPE only; the raw token must never reach the event. - "opik_mcp_at_FORBIDDEN-CANARY-oauth-token-2f8e1d3c", + f"{OAUTH_ACCESS_TOKEN_PREFIX}FORBIDDEN-CANARY-oauth-token-2f8e1d3c", ] @@ -879,11 +880,12 @@ def hi() -> str: ) # Drive the emit path directly with a canary-laden bearer token; the # event must carry only the bucketed reason/auth_mode, never the token. + bearer = f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}FORBIDDEN-CANARY-oauth-token-2f8e1d3c" scope = { "type": "http", "path": "/mcp", "headers": [ - (b"authorization", b"Bearer opik_mcp_at_FORBIDDEN-CANARY-oauth-token-2f8e1d3c"), + (b"authorization", bearer.encode()), ], } mw._emit_rejection(scope, 401) diff --git a/tests/test_http_auth.py b/tests/test_http_auth.py index 5bd035b..ef3b495 100644 --- a/tests/test_http_auth.py +++ b/tests/test_http_auth.py @@ -10,6 +10,8 @@ import httpx import pytest +from opik_mcp.auth_context import OAUTH_ACCESS_TOKEN_PREFIX + INITIALIZE = { "jsonrpc": "2.0", "id": 1, @@ -48,7 +50,7 @@ async def test_any_bearer_initializes(http_client: httpx.AsyncClient) -> None: "/mcp", json=INITIALIZE, headers={ - "Authorization": "Bearer opik_mcp_at_anything", + "Authorization": f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}anything", "Accept": "application/json, text/event-stream", }, ) diff --git a/tests/test_oauth_passthrough_mode.py b/tests/test_oauth_passthrough_mode.py index 300fa4a..44aea12 100644 --- a/tests/test_oauth_passthrough_mode.py +++ b/tests/test_oauth_passthrough_mode.py @@ -11,7 +11,11 @@ from starlette.requests import Request from starlette.responses import JSONResponse, Response -from opik_mcp.auth_context import inbound_authorization, inbound_workspace +from opik_mcp.auth_context import ( + OAUTH_ACCESS_TOKEN_PREFIX, + inbound_authorization, + inbound_workspace, +) from opik_mcp.server import BearerAuthMiddleware @@ -51,7 +55,7 @@ async def test_passthrough_accepts_any_well_formed_bearer() -> None: opik-backend's AuthFilter validates the token; opik-mcp is a thin pipe. """ mw = _build_middleware() - request = _make_request({"authorization": "Bearer opik_mcp_at_abc123"}) + request = _make_request({"authorization": f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc123"}) captured: dict[str, str | None] = {} @@ -64,7 +68,7 @@ async def call_next(_r: Request) -> Response: assert resp.status_code == 200 # Bearer captured exactly as inbound so outbound forwarding preserves it. - assert captured["auth"] == "Bearer opik_mcp_at_abc123" + assert captured["auth"] == f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc123" # No workspace header on this request → ContextVar reads as None. assert captured["workspace"] is None # ContextVar is reset after the request returns — no leakage to the @@ -78,7 +82,7 @@ async def test_passthrough_captures_comet_workspace_header() -> None: mw = _build_middleware() request = _make_request( { - "authorization": "Bearer opik_mcp_at_abc", + "authorization": f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc", "comet-workspace": "my-team", } ) diff --git a/tests/test_opik_client.py b/tests/test_opik_client.py index 6d7c21e..75b620d 100644 --- a/tests/test_opik_client.py +++ b/tests/test_opik_client.py @@ -2,6 +2,7 @@ import pytest import respx +from opik_mcp.auth_context import OAUTH_ACCESS_TOKEN_PREFIX from opik_mcp.opik_client import ( FeedbackScore, OpikAuthError, @@ -363,13 +364,13 @@ def test_resolve_opik_config_oauth_token_makes_workspace_optional() -> None: from opik_mcp.opik_client import resolve_opik_config s = Settings(opik_api_key=None, comet_workspace=None, opik_url="https://opik.example.com") - token = inbound_authorization.set("Bearer opik_mcp_at_abc123") + token = inbound_authorization.set(f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc123") try: _base, api_key, workspace = resolve_opik_config(s) finally: inbound_authorization.reset(token) - assert api_key == "Bearer opik_mcp_at_abc123" + assert api_key == f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc123" assert workspace is None @@ -383,7 +384,7 @@ def test_resolve_opik_config_oauth_detection_is_prefix_not_substring() -> None: from opik_mcp.opik_client import resolve_opik_config s = Settings(opik_api_key=None, comet_workspace=None, opik_url="https://opik.example.com") - token = inbound_authorization.set("Bearer sk-xopik_mcp_at_y") + token = inbound_authorization.set(f"Bearer sk-x{OAUTH_ACCESS_TOKEN_PREFIX}y") try: _base, _api_key, workspace = resolve_opik_config(s) finally: @@ -395,11 +396,13 @@ def test_oauth_client_omits_workspace_header() -> None: from opik_mcp.opik_client import OpikClient client = OpikClient( - base_url="https://opik.example.com", api_key="Bearer opik_mcp_at_x", workspace=None + base_url="https://opik.example.com", + api_key=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x", + workspace=None, ) headers = client._headers() assert "Comet-Workspace" not in headers - assert headers["Authorization"] == "Bearer opik_mcp_at_x" + assert headers["Authorization"] == f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x" # --- execute_experiment ------------------------------------------------------- #