diff --git a/changelog.d/sentry-expected-upstream-errors.md b/changelog.d/sentry-expected-upstream-errors.md new file mode 100644 index 000000000..a9cebdedb --- /dev/null +++ b/changelog.d/sentry-expected-upstream-errors.md @@ -0,0 +1,5 @@ +--- +category: Fixes +--- + +**Expected upstream provider errors no longer burn Sentry quota**: the Sentry Anthropic integration captures provider errors unhandled at the SDK call site, before the pipeline converts them into a `BackendAPIError` response. `_sentry_before_send` now drops throttling/availability errors (408, 429, 500, 502, 503, 504, 529) unconditionally. 400/404 — the client sent content or a model name Anthropic legitimately rejected — are dropped only when the request that reached Anthropic is proven to be exactly what the client sent (no policy hook rewrote or mutated it, no `UPSTREAM_HEADERS` template injected a header, no policy-context note was added). 401 requires that **plus** proof the forwarded credential was the client's own rather than the operator's shared `ANTHROPIC_API_KEY`: client-key auth mode always forwards the server's credential, so an unmodified body alone proves nothing about who caused a 401 in that mode, and an invalid operator credential must still report. Both provenance facts are tagged on the Sentry scope at the actual upstream call boundary (`_AnthropicPolicyIO`) as separate tags — `PASSTHROUGH_TAG` (body/headers) and `CREDENTIAL_PASSTHROUGH_TAG` (credential origin) — and checked per status code in `_sentry_before_send`, so a 400/401/404 caused by a proxy bug, a policy, or an invalid operator credential still reports instead of being mistaken for the client's or provider's problem. A status code outside this set (e.g. 403) still reports, so a new upstream failure mode stays visible. diff --git a/src/luthien_proxy/observability/sentry.py b/src/luthien_proxy/observability/sentry.py index e7c07852b..b3adf5ad6 100644 --- a/src/luthien_proxy/observability/sentry.py +++ b/src/luthien_proxy/observability/sentry.py @@ -2,16 +2,18 @@ Layer 1 (EventScrubber): strips values by key name (api_key, token, etc.) Layer 2 (before_send hook): summarizes LLM content variables with type+length, -strips cookies/server_name, redacts non-safe headers. +strips cookies/server_name, redacts non-safe headers, and drops expected +upstream provider errors. """ from __future__ import annotations import logging from itertools import islice -from typing import Any +from typing import Any, Mapping import sentry_sdk +from anthropic import APIStatusError from sentry_sdk.integrations.logging import ignore_logger from sentry_sdk.scrubber import DEFAULT_DENYLIST, EventScrubber from sentry_sdk.types import Event, Hint @@ -44,6 +46,101 @@ "raw_http_request", } +# Upstream statuses that mean the request/response is the client's or the +# provider's problem, not a proxy defect. The pipeline already converts every +# one of these into a BackendAPIError response for the client (see +# _handle_anthropic_error / _build_error_event, which log at warning and +# handle every AnthropicStatusError the same way regardless of status code) +# and, for the throttling/availability codes, the caller retries. They arrive +# here anyway because the Sentry Anthropic integration captures at the SDK +# call site with handled=false, before our handler ever sees them. + +# 408/429/500/502/503/504/529: provider throttling or brief unavailability. +# Structurally impossible for the proxy to have provoked — dropped +# unconditionally (56 unhandled 429 events in three days before this filter +# existed). +_PROVIDER_SIDE_STATUS_CODES = frozenset({408, 429, 500, 502, 503, 504, 529}) + +# 400/404: the client sent content Anthropic legitimately rejected — +# malformed message content or an unknown model name (LUTHIEN-6: 1,080 +# events for one recurring 400; LUTHIEN-2: unknown-model 404s). This is a +# property of the request body alone, independent of which credential +# reached Anthropic. But the proxy is not always a transparent passthrough: +# policy hooks can mutate or replace the request before it reaches +# Anthropic, and operator-configured UPSTREAM_HEADERS or policy-context +# injection can alter it too. Only drop these when the request carries +# provenance (the PASSTHROUGH_TAG scope tag, set at the actual upstream +# call boundary in _AnthropicPolicyIO) proving nothing touched it after the +# client sent it. +_CONTENT_DEPENDENT_STATUS_CODES = frozenset({400, 404}) + +# 401: an invalid bearer token passed through client-credential mode +# (LUTHIEN-D). Unlike 400/404, this is NOT solely a body/header property: in +# client-key auth mode the *credential* forwarded upstream is the operator's +# own ANTHROPIC_API_KEY rather than anything the client sent, so an +# unmodified body proves nothing about whose credential caused the 401 in +# that mode — an invalid operator credential must still report. Dropping a +# 401 requires BOTH the PASSTHROUGH_TAG (request untouched) AND the +# CREDENTIAL_PASSTHROUGH_TAG (credential is the client's own, not the +# operator's shared key). +_CREDENTIAL_DEPENDENT_STATUS_CODES = frozenset({401}) + +_CLIENT_OR_PASSTHROUGH_STATUS_CODES = _CONTENT_DEPENDENT_STATUS_CODES | _CREDENTIAL_DEPENDENT_STATUS_CODES + +_EXPECTED_UPSTREAM_STATUS_CODES = _PROVIDER_SIDE_STATUS_CODES | _CLIENT_OR_PASSTHROUGH_STATUS_CODES + +# Scope tag set by the Anthropic pipeline (see _AnthropicPolicyIO in +# anthropic_processor.py) immediately before the upstream call, true only +# when the request body and headers going to Anthropic are exactly what the +# client sent — no policy hook, header injection, or context injection +# touched them. Says nothing about which credential was forwarded; see +# CREDENTIAL_PASSTHROUGH_TAG for that. +PASSTHROUGH_TAG = "luthien.request_unmodified_passthrough" + +# Scope tag set alongside PASSTHROUGH_TAG, true only when the credential +# forwarded to Anthropic is the client's own (passthrough / BOTH / explicit +# x-anthropic-api-key auth) rather than the operator's shared +# ANTHROPIC_API_KEY substituted in client-key auth mode. Only 401 needs +# this — a bad body/model name (400/404) is credential-independent. +CREDENTIAL_PASSTHROUGH_TAG = "luthien.credential_client_supplied" + +# The decision table itself: which scope tags (all must be True) are required +# to drop each expected-upstream status code. Empty for the provider-side +# codes (unconditional), PASSTHROUGH_TAG alone for the content-dependent +# codes, both tags for the credential-dependent code. Built from the sets +# above so it cannot drift from the per-category documentation there, and a +# status absent from every set above is absent here too, so it always +# reports — see _is_expected_upstream_error. +_REQUIRED_TAGS_BY_STATUS: dict[int, tuple[str, ...]] = { + **dict.fromkeys(_PROVIDER_SIDE_STATUS_CODES, ()), + **dict.fromkeys(_CONTENT_DEPENDENT_STATUS_CODES, (PASSTHROUGH_TAG,)), + **dict.fromkeys(_CREDENTIAL_DEPENDENT_STATUS_CODES, (PASSTHROUGH_TAG, CREDENTIAL_PASSTHROUGH_TAG)), +} + + +def tag_request_provenance(unmodified: bool) -> None: + """Record on the current Sentry scope whether the outgoing request is untouched. + + Called at the upstream call boundary so `_sentry_before_send` can tell a + genuine client/provider 400/404 from one the proxy or a policy caused. + Safe to call even when Sentry is disabled or uninitialized — + `sentry_sdk.set_tag` is a no-op against the default scope in that case. + """ + sentry_sdk.set_tag(PASSTHROUGH_TAG, unmodified) + + +def tag_credential_provenance(client_supplied: bool) -> None: + """Record on the current Sentry scope whether the forwarded credential is the client's own. + + Called at the upstream call boundary alongside `tag_request_provenance` + so `_sentry_before_send` can tell a genuine client credential failure + (401) from an invalid operator credential in client-key auth mode. Safe + to call even when Sentry is disabled or uninitialized — + `sentry_sdk.set_tag` is a no-op against the default scope in that case. + """ + sentry_sdk.set_tag(CREDENTIAL_PASSTHROUGH_TAG, client_supplied) + + _SAFE_REQUEST_KEYS = {"model", "stream", "max_tokens", "temperature", "top_p", "top_k"} _SAFE_HEADERS = {"content-type", "accept", "user-agent", "x-request-id"} @@ -80,6 +177,23 @@ def _summarize(value: Any) -> Any: return f"<{type(value).__name__}>" +def _is_expected_upstream_error(exc: BaseException | None, tags: Mapping[str, object]) -> bool: + """True for provider errors that are the client's or provider's fault, not ours. + + Matches on the SDK exception's own status_code rather than its class so a + provider SDK renaming or adding a status subclass cannot silently start + reporting again. Looks up the required scope tags for that status in + _REQUIRED_TAGS_BY_STATUS and drops only when every one of them is True; + a status with no entry there always reports. + """ + if not isinstance(exc, APIStatusError): + return False + required_tags = _REQUIRED_TAGS_BY_STATUS.get(exc.status_code) + if required_tags is None: + return False + return all(tags.get(tag) is True for tag in required_tags) + + def _sentry_before_send(event: Event, hint: Hint) -> Event | None: """Selectively redact sensitive data while preserving debugging context. @@ -91,8 +205,11 @@ def _sentry_before_send(event: Event, hint: Hint) -> Event | None: drop the event entirely, or the (mutated) event to send it. """ exc_info = hint.get("exc_info") - if isinstance(exc_info, tuple) and exc_info[0] in {KeyboardInterrupt, SystemExit}: - return None + if isinstance(exc_info, tuple): + if exc_info[0] in {KeyboardInterrupt, SystemExit}: + return None + if _is_expected_upstream_error(exc_info[1], event.get("tags") or {}): + return None event.pop("server_name", None) diff --git a/src/luthien_proxy/pipeline/anthropic_processor.py b/src/luthien_proxy/pipeline/anthropic_processor.py index a479a7a21..572e9772f 100644 --- a/src/luthien_proxy/pipeline/anthropic_processor.py +++ b/src/luthien_proxy/pipeline/anthropic_processor.py @@ -50,6 +50,7 @@ build_usage, ) from luthien_proxy.observability.emitter import EventEmitterProtocol +from luthien_proxy.observability.sentry import tag_credential_provenance, tag_request_provenance from luthien_proxy.pipeline.client_format import ClientFormat from luthien_proxy.pipeline.policy_context_injection import inject_policy_awareness_anthropic from luthien_proxy.pipeline.session import ( @@ -110,10 +111,19 @@ def __init__( user_id: str | None, request_log_recorder: RequestLogRecorder, is_streaming: bool, + client_request_unmodified: bool, + credential_passthrough: bool, extra_headers: dict[str, str] | None = None, ) -> None: self._request = initial_request - self._initial_request = initial_request + # Deep-copied: policies are allowed to mutate the request dict + # in-place (see the identical rationale on _first_backend_response + # below) rather than replacing it via set_request(). A live + # reference here would silently "see" that mutation too, corrupting + # both this snapshot and the provenance check in + # _tag_request_provenance, which compares the request actually sent + # upstream against this value to detect policy-side modification. + self._initial_request = copy.deepcopy(initial_request) self._anthropic_client = anthropic_client self._emitter = emitter self._call_id = call_id @@ -122,6 +132,19 @@ def __init__( self._request_log_recorder = request_log_recorder self._is_streaming = is_streaming self._extra_headers = extra_headers + # Whether the pipeline had already changed the request/headers before + # policy hooks ever ran (context injection, UPSTREAM_HEADERS) — see + # process_anthropic_request. Combined with the initial_request + # comparison in _tag_provenance to decide passthrough provenance for + # Sentry (see observability/sentry.py:PASSTHROUGH_TAG). + self._client_request_unmodified = client_request_unmodified + # Whether the credential forwarded upstream is the client's own + # (passthrough / x-anthropic-api-key auth) rather than the + # operator's shared ANTHROPIC_API_KEY substituted in client-key auth + # mode — see process_anthropic_request. A 401 caused by an invalid + # *operator* credential must never be tagged CREDENTIAL_PASSTHROUGH_TAG + # just because the request body was untouched. + self._credential_passthrough = credential_passthrough self._request_recorded = False self._first_backend_response: AnthropicResponse | None = None # Raw backend events are only buffered when needed for non-streaming @@ -182,10 +205,27 @@ def _record_backend_request(self, request: AnthropicRequest) -> None: endpoint="/v1/messages", ) + def _tag_request_provenance(self, final_request: AnthropicRequest) -> None: + """Tag the Sentry scope with the request and credential provenance. + + `self._client_request_unmodified` covers pipeline-level changes made + before any policy hook ran (context injection, UPSTREAM_HEADERS); + comparing `final_request` against the pre-hook snapshot + (`self._initial_request`) covers what a policy hook did to it. Both + must hold for PASSTHROUGH_TAG (body/headers untouched). Credential + provenance is tagged separately via CREDENTIAL_PASSTHROUGH_TAG since + a bad body/model name (400/404) is credential-independent — only a + 401 needs both tags true (see observability/sentry.py). + """ + unmodified = self._client_request_unmodified and final_request == self._initial_request + tag_request_provenance(unmodified) + tag_credential_provenance(self._credential_passthrough) + async def complete(self, request: AnthropicRequest | None = None) -> AnthropicResponse: """Execute a non-streaming backend request.""" final_request = request or self._request self._record_backend_request(final_request) + self._tag_request_provenance(final_request) with tracer.start_as_current_span("send_upstream") as span: span.set_attribute("luthien.phase", "send_upstream") @@ -200,6 +240,7 @@ def stream(self, request: AnthropicRequest | None = None) -> AsyncIterator[Messa """Execute a streaming backend request.""" final_request = request or self._request self._record_backend_request(final_request) + self._tag_request_provenance(final_request) extra_headers = self._extra_headers @@ -424,13 +465,29 @@ async def process_anthropic_request( # Expand configurable upstream headers (e.g. Helicone session/auth headers). # Templates in UPSTREAM_HEADERS env var are expanded with per-request context. - forwarded_headers = merge_forwarded_headers( - base=forwarded_headers, - upstream=expand_upstream_headers( - session_id=session_id, - request_path=raw_http_request.path, - ), + upstream_injected_headers = expand_upstream_headers( + session_id=session_id, + request_path=raw_http_request.path, ) + forwarded_headers = merge_forwarded_headers(base=forwarded_headers, upstream=upstream_injected_headers) + + # Passthrough provenance so far (before policy hooks run): true only + # when neither policy-context injection (above) nor an + # operator-configured upstream header changed anything the client + # didn't itself send. `anthropic-beta` forwarding doesn't count + # against this — it relays a header the client already set, verbatim. + # _AnthropicPolicyIO combines this with what happens in policy hooks + # to decide the final Sentry PASSTHROUGH_TAG (observability/sentry.py). + client_request_unmodified = anthropic_request == raw_http_request.body and not upstream_injected_headers + + # Whether the credential Anthropic will see is the client's own, not + # the operator's shared ANTHROPIC_API_KEY. resolve_anthropic_client + # (gateway_routes.py) passes `user_credential=None` exactly when a + # client-key-mode request matched the shared key and the server's own + # base_client/credential is forwarded instead — that branch is never + # a client passthrough no matter how untouched the body is, since a + # 401 there means the *operator's* credential is invalid. + credential_passthrough = user_credential is not None # Create policy cache factory if database is available. The cap is # configured once here so every policy's cache honors the same limit; @@ -466,6 +523,8 @@ async def process_anthropic_request( is_streaming=is_streaming, root_span=root_span, request_log_recorder=request_log_recorder, + client_request_unmodified=client_request_unmodified, + credential_passthrough=credential_passthrough, extra_headers=forwarded_headers, usage_collector=usage_collector, webhook_sender=webhook_sender, @@ -620,6 +679,8 @@ async def _execute_anthropic_policy( root_span: Span, request_log_recorder: RequestLogRecorder, request_start_time: float, + client_request_unmodified: bool, + credential_passthrough: bool, extra_headers: dict[str, str] | None = None, usage_collector: UsageCollector | None = None, webhook_sender: WebhookSender | None = None, @@ -634,6 +695,8 @@ async def _execute_anthropic_policy( user_id=policy_ctx.user_id, request_log_recorder=request_log_recorder, is_streaming=is_streaming, + client_request_unmodified=client_request_unmodified, + credential_passthrough=credential_passthrough, extra_headers=extra_headers, ) emissions = _run_policy_hooks(execution_policy, io, policy_ctx) diff --git a/tests/luthien_proxy/unit_tests/pipeline/test_anthropic_processor.py b/tests/luthien_proxy/unit_tests/pipeline/test_anthropic_processor.py index 4df5a24bf..0a50d39c5 100644 --- a/tests/luthien_proxy/unit_tests/pipeline/test_anthropic_processor.py +++ b/tests/luthien_proxy/unit_tests/pipeline/test_anthropic_processor.py @@ -26,6 +26,7 @@ from tests.constants import DEFAULT_TEST_MODEL from tests.luthien_proxy.fixtures.policy_context import make_policy_context +from luthien_proxy.credentials import Credential, CredentialType from luthien_proxy.exceptions import BackendAPIError from luthien_proxy.llm.types.anthropic import AnthropicRequest, AnthropicResponse, build_usage from luthien_proxy.pipeline.anthropic_processor import ( @@ -646,6 +647,85 @@ async def test_non_streaming_request_end_to_end( assert isinstance(response, JSONResponse) mock_anthropic_client.complete.assert_called_once() + @pytest.mark.asyncio + async def test_client_key_mode_tags_credential_passthrough_false_despite_unmodified_body( + self, mock_request, mock_policy, mock_anthropic_client, mock_emitter + ): + """Route-level regression for the PR #809 credential-provenance finding. + + In client-key auth mode, resolve_anthropic_client (gateway_routes.py) + forwards the request with `user_credential=None` — the server's own + ANTHROPIC_API_KEY is used, not anything the client sent. Even with a + byte-identical, unmodified body, CREDENTIAL_PASSTHROUGH_TAG must be + False: an upstream 401 here means the *operator's* credential is + invalid, and dropping it from Sentry would silently hide the outage. + PASSTHROUGH_TAG (body/header provenance) is unaffected and still + tags True, since the body genuinely was untouched — 400/404 noise + reduction for client-key deployments must not regress. + """ + anthropic_body = { + "model": DEFAULT_TEST_MODEL, + "messages": [{"role": "user", "content": "Hello"}], + "max_tokens": 1024, + "stream": False, + } + mock_request.json = AsyncMock(return_value=anthropic_body) + + with ( + patch("luthien_proxy.pipeline.anthropic_processor.tracer"), + patch("luthien_proxy.pipeline.anthropic_processor.tag_request_provenance") as mock_body_tag, + patch("luthien_proxy.pipeline.anthropic_processor.tag_credential_provenance") as mock_cred_tag, + ): + await process_anthropic_request( + request=mock_request, + policy=mock_policy, + anthropic_client=mock_anthropic_client, + emitter=mock_emitter, + user_credential=None, + ) + + mock_body_tag.assert_called_once_with(True) + mock_cred_tag.assert_called_once_with(False) + + @pytest.mark.asyncio + async def test_passthrough_mode_with_unmodified_body_tags_both_true( + self, mock_request, mock_policy, mock_anthropic_client, mock_emitter + ): + """Sanity check: an unmodified body forwarded with the client's own + credential (passthrough / BOTH auth mode) is still a genuine + passthrough and must tag both PASSTHROUGH_TAG and + CREDENTIAL_PASSTHROUGH_TAG true — the credential check must not + overcorrect into always tagging False. + """ + anthropic_body = { + "model": DEFAULT_TEST_MODEL, + "messages": [{"role": "user", "content": "Hello"}], + "max_tokens": 1024, + "stream": False, + } + mock_request.json = AsyncMock(return_value=anthropic_body) + client_credential = Credential( + value="sk-ant-client-token", + credential_type=CredentialType.API_KEY, + platform="anthropic", + ) + + with ( + patch("luthien_proxy.pipeline.anthropic_processor.tracer"), + patch("luthien_proxy.pipeline.anthropic_processor.tag_request_provenance") as mock_body_tag, + patch("luthien_proxy.pipeline.anthropic_processor.tag_credential_provenance") as mock_cred_tag, + ): + await process_anthropic_request( + request=mock_request, + policy=mock_policy, + anthropic_client=mock_anthropic_client, + emitter=mock_emitter, + user_credential=client_credential, + ) + + mock_body_tag.assert_called_once_with(True) + mock_cred_tag.assert_called_once_with(True) + @pytest.mark.asyncio async def test_emits_transaction_request_recorded( self, mock_request, mock_policy, mock_anthropic_client, mock_emitter @@ -2084,6 +2164,8 @@ def _make_io(self, *, is_streaming: bool) -> _AnthropicPolicyIO: user_id=None, request_log_recorder=MagicMock(), is_streaming=is_streaming, + client_request_unmodified=True, + credential_passthrough=True, ) def test_buffer_raw_events_false_when_streaming(self): @@ -2122,6 +2204,169 @@ def test_non_streaming_uses_raw_backend_events(self): assert raw_events is io._raw_backend_events +class TestAnthropicPolicyIORequestProvenance: + """Tests for _AnthropicPolicyIO tagging Sentry with request/credential provenance. + + Covers PR #809 finding: 400/404 from Anthropic must only be treated as + "expected" (dropped from Sentry) when the request that reached Anthropic is + provably what the client sent — see observability/sentry.py:PASSTHROUGH_TAG. + Also covers the follow-up finding that PASSTHROUGH_TAG alone is not enough + for a 401: in client-key auth mode the *credential* forwarded upstream is + the operator's own ANTHROPIC_API_KEY rather than anything the client sent, + so CREDENTIAL_PASSTHROUGH_TAG is tagged separately and must NOT gate + 400/404 (which are credential-independent, and were PR #809's original + noise-reduction target for client-key deployments). + """ + + def _make_io( + self, + *, + request: AnthropicRequest, + client_request_unmodified: bool = True, + credential_passthrough: bool = True, + ) -> _AnthropicPolicyIO: + return _AnthropicPolicyIO( + initial_request=request, + anthropic_client=MagicMock(), + emitter=MagicMock(), + call_id="test-call", + session_id=None, + user_id=None, + request_log_recorder=MagicMock(), + is_streaming=False, + client_request_unmodified=client_request_unmodified, + credential_passthrough=credential_passthrough, + ) + + @pytest.mark.asyncio + async def test_unmodified_request_tags_passthrough_true(self): + """A request nothing touched must tag True.""" + request: AnthropicRequest = { + "model": DEFAULT_TEST_MODEL, + "max_tokens": 100, + "messages": [{"role": "user", "content": "hi"}], + } + io = self._make_io(request=request) + io._anthropic_client.complete = AsyncMock(return_value={"id": "msg_1"}) + + with patch("luthien_proxy.pipeline.anthropic_processor.tag_request_provenance") as mock_tag: + await io.complete(request) + + mock_tag.assert_called_once_with(True) + + @pytest.mark.asyncio + async def test_policy_replaced_request_tags_passthrough_false(self): + """A policy hook returning a different request object must tag False.""" + request: AnthropicRequest = { + "model": DEFAULT_TEST_MODEL, + "max_tokens": 100, + "messages": [{"role": "user", "content": "hi"}], + } + io = self._make_io(request=request) + io._anthropic_client.complete = AsyncMock(return_value={"id": "msg_1"}) + + replaced_request: AnthropicRequest = {**request, "max_tokens": 999} + io.set_request(replaced_request) + + with patch("luthien_proxy.pipeline.anthropic_processor.tag_request_provenance") as mock_tag: + await io.complete(replaced_request) + + mock_tag.assert_called_once_with(False) + + @pytest.mark.asyncio + async def test_policy_mutated_request_in_place_tags_passthrough_false(self): + """A policy hook mutating the SAME dict in place (rather than replacing + it) must still be detected — _initial_request is a deep copy exactly to + guard against this aliasing corrupting the comparison (matches + _AddMaxTokensPolicy's mutate-and-return style in TestRunPolicyHooks). + """ + request: AnthropicRequest = { + "model": DEFAULT_TEST_MODEL, + "max_tokens": 100, + "messages": [{"role": "user", "content": "hi"}], + } + io = self._make_io(request=request) + io._anthropic_client.complete = AsyncMock(return_value={"id": "msg_1"}) + + request["max_tokens"] = 999 + io.set_request(request) + + with patch("luthien_proxy.pipeline.anthropic_processor.tag_request_provenance") as mock_tag: + await io.complete(request) + + mock_tag.assert_called_once_with(False) + + @pytest.mark.asyncio + async def test_pipeline_level_modification_tags_passthrough_false_even_if_hooks_are_noop(self): + """client_request_unmodified=False (context injection / UPSTREAM_HEADERS + happened before hooks ran) must tag False even when no policy hook + changes anything further. + """ + request: AnthropicRequest = { + "model": DEFAULT_TEST_MODEL, + "max_tokens": 100, + "messages": [{"role": "user", "content": "hi"}], + } + io = self._make_io(request=request, client_request_unmodified=False) + io._anthropic_client.complete = AsyncMock(return_value={"id": "msg_1"}) + + with patch("luthien_proxy.pipeline.anthropic_processor.tag_request_provenance") as mock_tag: + await io.complete(request) + + mock_tag.assert_called_once_with(False) + + @pytest.mark.asyncio + async def test_client_key_mode_tags_credential_passthrough_false_even_if_body_unmodified(self): + """credential_passthrough=False (client-key auth mode, server's shared + ANTHROPIC_API_KEY forwarded instead of anything the client sent) must + tag CREDENTIAL_PASSTHROUGH_TAG False even when the body and headers + are completely untouched — an upstream 401 in that mode means the + *operator's* credential is invalid, not the client's, and must not be + dropped from Sentry. PASSTHROUGH_TAG (body/header provenance) is + unaffected and still tags True, since the body genuinely was + untouched. + """ + request: AnthropicRequest = { + "model": DEFAULT_TEST_MODEL, + "max_tokens": 100, + "messages": [{"role": "user", "content": "hi"}], + } + io = self._make_io(request=request, credential_passthrough=False) + io._anthropic_client.complete = AsyncMock(return_value={"id": "msg_1"}) + + with ( + patch("luthien_proxy.pipeline.anthropic_processor.tag_request_provenance") as mock_body_tag, + patch("luthien_proxy.pipeline.anthropic_processor.tag_credential_provenance") as mock_cred_tag, + ): + await io.complete(request) + + mock_body_tag.assert_called_once_with(True) + mock_cred_tag.assert_called_once_with(False) + + @pytest.mark.asyncio + async def test_stream_tags_passthrough_based_on_same_rules(self): + """stream() must apply the identical provenance rule as complete().""" + request: AnthropicRequest = { + "model": DEFAULT_TEST_MODEL, + "max_tokens": 100, + "messages": [{"role": "user", "content": "hi"}], + "stream": True, + } + io = self._make_io(request=request) + + async def _fake_stream(req, extra_headers=None): + return + yield # pragma: no cover - makes this an async generator + + io._anthropic_client.stream = _fake_stream + + with patch("luthien_proxy.pipeline.anthropic_processor.tag_request_provenance") as mock_tag: + async for _ in io.stream(request): + pass + + mock_tag.assert_called_once_with(True) + + class TestStreamingWebhookGate: """Tests for the streaming webhook fire gate (streaming completion gate). diff --git a/tests/luthien_proxy/unit_tests/test_sentry_scrubbing.py b/tests/luthien_proxy/unit_tests/test_sentry_scrubbing.py index 5e9646295..7eaa421b6 100644 --- a/tests/luthien_proxy/unit_tests/test_sentry_scrubbing.py +++ b/tests/luthien_proxy/unit_tests/test_sentry_scrubbing.py @@ -4,7 +4,15 @@ import pytest -from luthien_proxy.observability.sentry import _sentry_before_send, _summarize +from luthien_proxy.observability.sentry import ( + _CONTENT_DEPENDENT_STATUS_CODES, + _CREDENTIAL_DEPENDENT_STATUS_CODES, + _PROVIDER_SIDE_STATUS_CODES, + CREDENTIAL_PASSTHROUGH_TAG, + PASSTHROUGH_TAG, + _sentry_before_send, + _summarize, +) pytestmark = pytest.mark.timeout(10) @@ -88,9 +96,12 @@ def _make_event( include_cookies=True, include_frame_vars=True, frame_vars_empty=False, + tags=None, ): """Build a realistic Sentry event for testing.""" event = {} + if tags is not None: + event["tags"] = tags if include_server_name: event["server_name"] = "gateway-prod-123" @@ -175,6 +186,193 @@ def test_non_tuple_exc_info_does_not_crash(self): result = _sentry_before_send(event, {"exc_info": "not-a-tuple"}) assert result is not None + def _status_error(self, status_code: int): + """Build a real Anthropic status error, as the SDK raises it.""" + import httpx + from anthropic import APIStatusError + + request = httpx.Request("POST", "https://api.anthropic.com/v1/messages") + response = httpx.Response(status_code, request=request, json={"error": {"message": "upstream"}}) + return APIStatusError("upstream", response=response, body=None) + + def test_drops_upstream_rate_limit_error(self): + """A 429 from Anthropic is the upstream telling us to slow down, not a proxy + bug. The SDK integration captures it unhandled at the call site, which burned + 56 events in three days and buries real failures.""" + exc = self._status_error(429) + event = self._make_event() + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is None + + def test_drops_upstream_overloaded_error(self): + exc = self._status_error(529) + event = self._make_event() + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is None + + def test_drops_upstream_bad_request_error(self): + """A 400 means the client sent content Anthropic rejects (e.g. an unsupported + field). Dropped only when the request carrying the PASSTHROUGH_TAG proves the + proxy relayed it unchanged — see LUTHIEN-6, 1,080 events for one recurring case.""" + exc = self._status_error(400) + event = self._make_event(tags={PASSTHROUGH_TAG: True}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is None + + def test_drops_bad_request_error_regardless_of_credential_tag(self): + """400 is credential-independent: a client-key-mode request (server's + shared ANTHROPIC_API_KEY forwarded, CREDENTIAL_PASSTHROUGH_TAG False) + with an unmodified body must still drop a 400 — the operator's + credential has nothing to do with a malformed message the client + sent. Regression guard for the credential-provenance fix + overcorrecting into gating 400/404 too.""" + exc = self._status_error(400) + event = self._make_event(tags={PASSTHROUGH_TAG: True, CREDENTIAL_PASSTHROUGH_TAG: False}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is None + + def test_drops_upstream_not_found_error(self): + """A 404 means the client asked for a model Anthropic doesn't have (LUTHIEN-2), + and the PASSTHROUGH_TAG proves the proxy didn't rewrite the request.""" + exc = self._status_error(404) + event = self._make_event(tags={PASSTHROUGH_TAG: True}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is None + + def test_drops_not_found_error_regardless_of_credential_tag(self): + """Same regression guard as the 400 case: an unknown model name is + credential-independent, so a 404 still drops in client-key mode.""" + exc = self._status_error(404) + event = self._make_event(tags={PASSTHROUGH_TAG: True, CREDENTIAL_PASSTHROUGH_TAG: False}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is None + + def test_drops_upstream_authentication_error(self): + """A 401 means the credential passed through to Anthropic was invalid + (LUTHIEN-D) — the proxy correctly forwarded a bad token, it didn't mint + one. Dropping requires BOTH PASSTHROUGH_TAG (request untouched) AND + CREDENTIAL_PASSTHROUGH_TAG (the forwarded credential was the client's + own, not the operator's shared key).""" + exc = self._status_error(401) + event = self._make_event(tags={PASSTHROUGH_TAG: True, CREDENTIAL_PASSTHROUGH_TAG: True}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is None + + def test_keeps_client_key_mode_authentication_error(self): + """A 401 with an unmodified body (PASSTHROUGH_TAG True) but the + operator's shared credential forwarded instead of the client's own + (CREDENTIAL_PASSTHROUGH_TAG False, client-key auth mode) must still + report — the operator's credential is invalid, not the client's, and + dropping it would silently hide the outage. This is the deep-review + finding that PR #809's original PASSTHROUGH_TAG-only check missed.""" + exc = self._status_error(401) + event = self._make_event(tags={PASSTHROUGH_TAG: True, CREDENTIAL_PASSTHROUGH_TAG: False}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is not None + + def test_keeps_authentication_error_when_credential_tag_absent(self): + """No CREDENTIAL_PASSTHROUGH_TAG at all (e.g. the tag was never set) + must fail closed for a 401 — absence of proof is not proof of + passthrough, same rule PASSTHROUGH_TAG already follows.""" + exc = self._status_error(401) + event = self._make_event(tags={PASSTHROUGH_TAG: True}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is not None + + def test_keeps_proxy_modified_bad_request_error(self): + """A 400 where the request was NOT proven to be an unmodified client + passthrough (PASSTHROUGH_TAG missing or False) must still report — a + policy hook or header/context injection could have built the invalid + request that Anthropic rejected, and that's a proxy bug (thermonuclear + review finding: consensus High on PR #809).""" + exc = self._status_error(400) + event = self._make_event(tags={PASSTHROUGH_TAG: False}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is not None + + def test_keeps_proxy_modified_not_found_error(self): + """Same as the 400 case: a 404 without proven passthrough stays visible.""" + exc = self._status_error(404) + event = self._make_event(tags={PASSTHROUGH_TAG: False}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is not None + + def test_keeps_proxy_modified_authentication_error(self): + """Same as the 400 case: a 401 without proven passthrough stays visible.""" + exc = self._status_error(401) + event = self._make_event(tags={PASSTHROUGH_TAG: False}) + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is not None + + def test_keeps_bad_request_error_when_passthrough_tag_absent(self): + """No PASSTHROUGH_TAG at all (e.g. the tag was never set) must fail + closed — absence of proof is not proof of passthrough.""" + exc = self._status_error(400) + event = self._make_event() + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is not None + + # ------------------------------------------------------------------ + # Decision-table coverage. Parametrized directly off the production + # status-code sets in observability/sentry.py, so a status silently + # added to (or removed from) one of those sets changes which cases these + # tests run without anyone having to remember to update a hand-written + # list here. The named tests above stay as regression pins for specific + # incidents; these supplement them with exhaustive tag-combination + # coverage. + # ------------------------------------------------------------------ + + _TAG_STATES = (True, False, None) # None means the tag is absent entirely + + def _tags_for(self, passthrough: bool | None, credential: bool | None) -> dict[str, object]: + tags: dict[str, object] = {} + if passthrough is not None: + tags[PASSTHROUGH_TAG] = passthrough + if credential is not None: + tags[CREDENTIAL_PASSTHROUGH_TAG] = credential + return tags + + @pytest.mark.parametrize("status", sorted(_PROVIDER_SIDE_STATUS_CODES)) + def test_decision_table_provider_side_drops_with_no_tags(self, status): + """Every status in _PROVIDER_SIDE_STATUS_CODES drops unconditionally — + no PASSTHROUGH_TAG or CREDENTIAL_PASSTHROUGH_TAG needed. Parametrized + from the set itself, so adding a status there without exercising it + here is not possible.""" + exc = self._status_error(status) + event = self._make_event() + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is None + + @pytest.mark.parametrize("status", sorted(_CONTENT_DEPENDENT_STATUS_CODES)) + @pytest.mark.parametrize("credential", _TAG_STATES, ids=lambda v: f"credential={v}") + @pytest.mark.parametrize("passthrough", _TAG_STATES, ids=lambda v: f"passthrough={v}") + def test_decision_table_content_dependent(self, passthrough, credential, status): + """400/404 drop iff PASSTHROUGH_TAG is True; the credential tag never + matters, in every one of the 9 tag combinations — pinning the + deliberate fix that content-only statuses don't gate on credential + provenance.""" + exc = self._status_error(status) + event = self._make_event(tags=self._tags_for(passthrough, credential)) + result = _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) + if passthrough is True: + assert result is None + else: + assert result is not None + + @pytest.mark.parametrize("status", sorted(_CREDENTIAL_DEPENDENT_STATUS_CODES)) + @pytest.mark.parametrize("credential", _TAG_STATES, ids=lambda v: f"credential={v}") + @pytest.mark.parametrize("passthrough", _TAG_STATES, ids=lambda v: f"passthrough={v}") + def test_decision_table_credential_dependent(self, passthrough, credential, status): + """401 drops only when BOTH PASSTHROUGH_TAG and CREDENTIAL_PASSTHROUGH_TAG + are True; every other one of the 9 combinations — including either + tag simply missing — must fail closed and still report.""" + exc = self._status_error(status) + event = self._make_event(tags=self._tags_for(passthrough, credential)) + result = _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) + if passthrough is True and credential is True: + assert result is None + else: + assert result is not None + + def test_keeps_upstream_status_code_outside_expected_set(self): + """A status code we have not classified as expected (e.g. 403) still reports, + so a new upstream failure mode is visible until someone evaluates it.""" + exc = self._status_error(403) + event = self._make_event() + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is not None + + def test_keeps_proxy_bugs(self): + """An ordinary exception from our own code must still report.""" + exc = TypeError("Object of type datetime is not JSON serializable") + event = self._make_event() + assert _sentry_before_send(event, {"exc_info": (type(exc), exc, None)}) is not None + def test_strips_server_name(self): event = self._make_event() hint = {}