fix(anthropic): fail closed when the SDK Omit sentinel is unavailable
The api-key credential-isolation guard in this PR only removes the env-derived Bearer when `anthropic._types.Omit` can be imported. When the sentinel is unavailable (older/exotic SDK layout), `_new_sdk_client` fell through to a bare api-key client whose SDK env fallback re-reads ANTHROPIC_AUTH_TOKEN and ships `Authorization: Bearer ***` to third-party endpoints — the compatibility path failed open (P2, reported by @egilewski). Fail closed instead: when Omit is unavailable, build the client with a copy-safe httpx request hook that strips Authorization on every request. `http_client` propagates through `with_options()`/`copy()`, so the original client and every copy are covered without depending on the SDK's header-omission internals — the same custom-client shape already used by `_build_anthropic_client_with_bearer_hook`. Adds a real-loopback e2e test that simulates Omit unavailable and asserts no Bearer leak on the original client and on a `with_options()` copy, while x-api-key and request success are preserved. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -352,6 +352,24 @@ def _sdk_omit_sentinel(sdk) -> Optional[Any]:
|
||||
return Omit()
|
||||
|
||||
|
||||
def _header_stripping_http_client(kwargs: Dict[str, Any], header: str):
|
||||
"""An ``httpx.Client`` that deletes ``header`` from every outgoing request. Copy-safe
|
||||
fallback for :func:`_new_sdk_client` when the SDK's ``Omit`` sentinel is unavailable: the
|
||||
SDK copies ``http_client`` into every ``with_options()``/``copy()`` clone (see its
|
||||
``copy()``), so the strip applies to the original client and all copies, with no dependency
|
||||
on the SDK's header-omission internals. Mirrors the custom-client pattern already used by
|
||||
:func:`_build_anthropic_client_with_bearer_hook`."""
|
||||
import httpx
|
||||
target = header.lower()
|
||||
|
||||
def _strip(request: "httpx.Request") -> None:
|
||||
# httpx headers are case-insensitive; pop by any casing the SDK wrote.
|
||||
for name in [k for k in request.headers if k.lower() == target]:
|
||||
del request.headers[name]
|
||||
|
||||
return httpx.Client(timeout=kwargs.get("timeout"), event_hooks={"request": [_strip]})
|
||||
|
||||
|
||||
def _new_sdk_client(sdk, kwargs: Dict[str, Any], headers: Dict[str, str]):
|
||||
"""``sdk.Anthropic(**kwargs)`` with ``headers`` attached. Bearer-only construction leaves
|
||||
``api_key`` unset, so the SDK fills it from ANTHROPIC_API_KEY (loaded from ~/.hermes/.env) and
|
||||
@@ -359,7 +377,7 @@ def _new_sdk_client(sdk, kwargs: Dict[str, Any], headers: Dict[str, str]):
|
||||
request; clear it whenever we intentionally authenticated via auth_token. Api-key-only
|
||||
construction has the mirror problem: the SDK fills ``auth_token`` from ANTHROPIC_AUTH_TOKEN in
|
||||
the environment and ships that Bearer credential to third-party Anthropic-compatible endpoints
|
||||
alongside x-api-key — clear it whenever we intentionally authenticated via api_key."""
|
||||
alongside x-api-key — suppress it whenever we intentionally authenticated via api_key."""
|
||||
if headers:
|
||||
kwargs["default_headers"] = headers
|
||||
if "api_key" in kwargs and "auth_token" not in kwargs:
|
||||
@@ -372,6 +390,12 @@ def _new_sdk_client(sdk, kwargs: Dict[str, Any], headers: Dict[str, str]):
|
||||
merged = dict(kwargs.get("default_headers") or {})
|
||||
merged["Authorization"] = omit
|
||||
kwargs["default_headers"] = merged
|
||||
elif "http_client" not in kwargs:
|
||||
# Omit unavailable (old/exotic SDK): do NOT fall through to a client that leaks the
|
||||
# env-derived Bearer. Fail closed onto a copy-safe request hook that strips
|
||||
# Authorization on the wire — http_client propagates through with_options()/copy(),
|
||||
# so the original client and every copy are covered.
|
||||
kwargs["http_client"] = _header_stripping_http_client(kwargs, "Authorization")
|
||||
client = sdk.Anthropic(**kwargs)
|
||||
if "auth_token" in kwargs and "api_key" not in kwargs:
|
||||
client.api_key = None
|
||||
|
||||
@@ -1904,6 +1904,45 @@ def _final_request_options(anthropic_sdk):
|
||||
return FinalRequestOptions(method="post", url="/v1/messages", json_data={})
|
||||
|
||||
|
||||
def _start_header_capturing_server():
|
||||
"""Local HTTP server that records the headers of each POST and returns a minimal Messages
|
||||
response. Returns ``(server, captured)``; the caller derives the base_url from
|
||||
``server.server_port`` and must call ``server.shutdown()``. Used by the fail-closed fallback
|
||||
tests, where the strip happens at the httpx transport layer (not in ``_build_headers``) and so
|
||||
can only be observed on a real request."""
|
||||
import json as _json
|
||||
import threading
|
||||
from http.server import BaseHTTPRequestHandler, HTTPServer
|
||||
|
||||
captured = {}
|
||||
|
||||
class _Handler(BaseHTTPRequestHandler):
|
||||
def do_POST(self):
|
||||
captured["headers"] = {k.lower(): v for k, v in self.headers.items()}
|
||||
self.rfile.read(int(self.headers.get("content-length", 0)))
|
||||
body = _json.dumps({
|
||||
"id": "msg_test",
|
||||
"type": "message",
|
||||
"role": "assistant",
|
||||
"content": [{"type": "text", "text": "ok"}],
|
||||
"model": "test",
|
||||
"stop_reason": "end_turn",
|
||||
"usage": {"input_tokens": 1, "output_tokens": 1},
|
||||
}).encode()
|
||||
self.send_response(200)
|
||||
self.send_header("content-type", "application/json")
|
||||
self.send_header("content-length", str(len(body)))
|
||||
self.end_headers()
|
||||
self.wfile.write(body)
|
||||
|
||||
def log_message(self, *args):
|
||||
pass
|
||||
|
||||
server = HTTPServer(("127.0.0.1", 0), _Handler)
|
||||
threading.Thread(target=server.serve_forever, daemon=True).start()
|
||||
return server, captured
|
||||
|
||||
|
||||
class TestApiKeyConstructionClearsEnvBearerToken:
|
||||
"""Api-key-style clients must not inherit ANTHROPIC_AUTH_TOKEN from the environment.
|
||||
|
||||
@@ -2052,3 +2091,32 @@ class TestApiKeyConstructionClearsEnvBearerToken:
|
||||
headers = captured["headers"]
|
||||
assert headers.get("x-api-key") == "third-party-provider-key"
|
||||
assert "sentinel-env-token-DO-NOT-SEND" not in headers.get("authorization", "")
|
||||
|
||||
def test_api_key_style_fails_closed_when_omit_unavailable(self, monkeypatch):
|
||||
"""When ``anthropic._types.Omit`` cannot be imported, the copy-safe Omit default header
|
||||
is unavailable — but the api-key path must still not leak ANTHROPIC_AUTH_TOKEN. It falls
|
||||
back to a request hook that strips Authorization on the wire (the strip is at the httpx
|
||||
transport layer, hence the real request), on the original client AND on a with_options()
|
||||
copy, while x-api-key and request success are preserved."""
|
||||
anthropic_sdk = pytest.importorskip("anthropic")
|
||||
monkeypatch.setenv("ANTHROPIC_AUTH_TOKEN", "sentinel-env-token-DO-NOT-SEND")
|
||||
monkeypatch.setattr("agent.anthropic_adapter._sdk_omit_sentinel", lambda sdk: None)
|
||||
|
||||
server, captured = _start_header_capturing_server()
|
||||
try:
|
||||
client = build_anthropic_client(
|
||||
"third-party-provider-key",
|
||||
base_url=f"http://127.0.0.1:{server.server_port}",
|
||||
)
|
||||
for wire_client in (client, client.with_options(timeout=30)):
|
||||
captured.clear()
|
||||
wire_client.messages.create(
|
||||
model="test-model",
|
||||
max_tokens=8,
|
||||
messages=[{"role": "user", "content": "hi"}],
|
||||
)
|
||||
headers = captured["headers"]
|
||||
assert headers.get("x-api-key") == "third-party-provider-key"
|
||||
assert "sentinel-env-token-DO-NOT-SEND" not in headers.get("authorization", "")
|
||||
finally:
|
||||
server.shutdown()
|
||||
|
||||
Reference in New Issue
Block a user