diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index acd22d884855..1838c23694c5 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -3615,6 +3615,13 @@ def _is_unsupported_parameter_error(exc: Exception, param: str) -> bool: "unrecognized request argument", "unrecognized parameter", "invalid parameter", + # AWS Bedrock Converse phrasing for newer Anthropic models + # ("`temperature` is deprecated for this model.") — without this the + # reactive-retry branch never fires and the boto3 ValidationException + # is swallowed by downstream wrappers, surfacing as an empty + # ChatCompletion with all-None fields. Ref: aux vision broken on + # Bedrock Opus 4.7 / Sonnet 4.5 when temperature is passed. + "is deprecated", )) @@ -5857,12 +5864,28 @@ def _wrap_if_needed(client_obj, final_model_str: str, base_url_str: str = "", else (client, final_model)) elif pconfig.auth_type == "aws_sdk": - # AWS SDK providers (Bedrock) — Claude models use the Anthropic Bedrock - # SDK (prompt caching, thinking); non-Claude models use Converse API. + # AWS SDK providers (Bedrock). Two auth paths: + # + # 1. AWS_BEARER_TOKEN_BEDROCK → boto3 Converse API via + # BedrockAuxiliaryClient, for BOTH Claude and non-Claude models. + # The ``anthropic.AnthropicBedrock`` SDK does NOT support bearer + # tokens — it raises ``RuntimeError: could not resolve credentials + # from session`` because its auth helper only consults the boto3 + # credential chain for IAM keys, not bearer tokens. boto3's + # Converse call DOES pick up the bearer token natively, and + # Converse supports every Bedrock model including Claude, so we + # route Claude through it too here. This mirrors the dual-path + # routing in ``hermes_cli.runtime_provider`` for the main loop. + # + # 2. IAM credentials (env vars / SSO / instance profile) → Claude + # models use the AnthropicBedrock SDK for full feature parity + # (prompt caching, thinking budgets); non-Claude models use the + # Converse shim. try: from agent.bedrock_adapter import ( has_aws_credentials, is_anthropic_bedrock_model, + resolve_aws_auth_env_var, resolve_bedrock_region, ) from agent.anthropic_adapter import build_anthropic_bedrock_client @@ -5878,9 +5901,20 @@ def _wrap_if_needed(client_obj, final_model_str: str, base_url_str: str = "", region = resolve_bedrock_region() default_model = "anthropic.claude-haiku-4-5-20251001-v1:0" - final_model = _normalize_resolved_model(model or default_model, provider) + final_model = _normalize_resolved_model(model or default_model, provider) or default_model base_url = f"https://bedrock-runtime.{region}.amazonaws.com" + # Bearer-token path: route everything (Claude included) through the + # boto3 Converse shim, which is the only path that can use the token. + if resolve_aws_auth_env_var() == "AWS_BEARER_TOKEN_BEDROCK": + client = BedrockAuxiliaryClient(region, final_model) + logger.debug( + "resolve_provider_client: bedrock converse (%s, %s, bearer-token)", + final_model, region, + ) + return (_to_async_client(client, final_model, is_vision=is_vision) if async_mode + else (client, final_model)) + if is_anthropic_bedrock_model(final_model): try: real_client = build_anthropic_bedrock_client(region) diff --git a/agent/bedrock_adapter.py b/agent/bedrock_adapter.py index c399081619ff..25b77a22fa73 100644 --- a/agent/bedrock_adapter.py +++ b/agent/bedrock_adapter.py @@ -579,11 +579,21 @@ def _convert_content_to_converse(content) -> List[Dict]: # wire layer, so passing the base64 string directly # results in double-encoding and Bedrock rejects it with # "Failed to sanitize image". Ref: #33317. + # + # ``validate=True`` makes b64decode reject payloads with + # characters outside the base64 alphabet instead of + # silently discarding them and returning junk bytes. On a + # malformed data URL we skip the image rather than sending + # the raw base64 string as bytes (which Bedrock rejects + # with ValidationException) or blowing up the whole request. import base64 + import binascii try: - raw_bytes = base64.b64decode(data) - except Exception: - raw_bytes = data.encode("utf-8") + raw_bytes = base64.b64decode(data, validate=True) + except (binascii.Error, ValueError): + continue + if not raw_bytes: + continue blocks.append({ "image": { "format": media_type.split("/")[-1] if "/" in media_type else "jpeg", diff --git a/tests/agent/test_bedrock_adapter.py b/tests/agent/test_bedrock_adapter.py index 8994688e0f21..5ae933e3a8bf 100644 --- a/tests/agent/test_bedrock_adapter.py +++ b/tests/agent/test_bedrock_adapter.py @@ -1104,16 +1104,34 @@ def test_data_url_decoded_to_bytes(self): assert isinstance(img_block["source"]["bytes"], bytes) assert img_block["source"]["bytes"] == raw_png - def test_invalid_base64_falls_back_to_encode(self): + def test_invalid_base64_skipped(self): from agent.bedrock_adapter import _convert_content_to_converse + # Malformed base64 (chars outside the alphabet) must be skipped, not + # forwarded as raw string-bytes — Bedrock rejects junk bytes with + # ValidationException. Skipping degrades gracefully (drop the image) + # instead of failing the whole request. data_url = "data:image/jpeg;base64,NOT_VALID_BASE64!!!" content = [{"type": "image_url", "image_url": {"url": data_url}}] blocks = _convert_content_to_converse(content) - # Should not crash — falls back to encoding the string as bytes - assert len(blocks) == 1 - assert isinstance(blocks[0]["image"]["source"]["bytes"], bytes) + # Malformed image is dropped — no image block produced. + assert all("image" not in b for b in blocks) + + def test_valid_image_survives_alongside_invalid(self): + from agent.bedrock_adapter import _convert_content_to_converse + + good = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR42mP8/5+hHgAHggJ/PchI7wAAAABJRU5ErkJggg==" + bad = "data:image/jpeg;base64,NOT_VALID_BASE64!!!" + content = [ + {"type": "image_url", "image_url": {"url": good}}, + {"type": "image_url", "image_url": {"url": bad}}, + ] + blocks = _convert_content_to_converse(content) + image_blocks = [b for b in blocks if "image" in b] + # Exactly the one valid image survives. + assert len(image_blocks) == 1 + assert isinstance(image_blocks[0]["image"]["source"]["bytes"], bytes) class TestBearerTokenRoutesToConverse: @@ -1157,3 +1175,50 @@ def test_sigv4_claude_still_uses_anthropic_bedrock_sdk(self, monkeypatch): runtime = self._resolve(monkeypatch, bearer=False) assert runtime["api_mode"] == "anthropic_messages" assert runtime.get("bedrock_anthropic") is True + + +class TestBearerTokenAuxRoutesToConverse: + """The AUXILIARY client (vision/summarization) must also route bearer-token + Claude models through the boto3 Converse shim (BedrockAuxiliaryClient), not + the AnthropicBedrock SDK — which raises ``RuntimeError: could not resolve + credentials from session`` on bearer tokens. This mirrors the main-loop + routing tested in TestBearerTokenRoutesToConverse. Ref: #28085. + """ + + def _resolve_aux(self, monkeypatch, model): + import agent.auxiliary_client as ac + + # bedrock is auth_type=aws_sdk in the real PROVIDER_REGISTRY, and with + # the bearer env set has_aws_credentials() is True — no registry mock + # needed. Guard build_anthropic_bedrock_client so a regression that + # routes bearer-token Claude back to the AnthropicBedrock SDK fails loud. + monkeypatch.setenv("AWS_BEARER_TOKEN_BEDROCK", "test-bearer-token-123") + monkeypatch.setenv("AWS_DEFAULT_REGION", "us-east-1") + + from agent import anthropic_adapter + def _boom(*a, **k): + raise AssertionError( + "build_anthropic_bedrock_client called on bearer-token path — " + "should route to Converse instead" + ) + monkeypatch.setattr(anthropic_adapter, "build_anthropic_bedrock_client", _boom, raising=False) + + return ac.resolve_provider_client(provider="bedrock", model=model) + + def test_bearer_claude_aux_uses_converse_client(self, monkeypatch): + from agent.auxiliary_client import BedrockAuxiliaryClient, AnthropicAuxiliaryClient + + client, final_model = self._resolve_aux( + monkeypatch, "anthropic.claude-opus-4-20250514-v1:0" + ) + # Claude + bearer token → Converse shim, NOT AnthropicAuxiliaryClient. + assert isinstance(client, BedrockAuxiliaryClient) + assert not isinstance(client, AnthropicAuxiliaryClient) + + def test_bearer_nonclaude_aux_uses_converse_client(self, monkeypatch): + from agent.auxiliary_client import BedrockAuxiliaryClient + + client, final_model = self._resolve_aux( + monkeypatch, "amazon.nova-pro-v1:0" + ) + assert isinstance(client, BedrockAuxiliaryClient) diff --git a/tests/agent/test_bedrock_integration.py b/tests/agent/test_bedrock_integration.py index 6f6fcbd8e08a..12aa478620bd 100644 --- a/tests/agent/test_bedrock_integration.py +++ b/tests/agent/test_bedrock_integration.py @@ -370,9 +370,13 @@ class TestAuxiliaryClientBedrockResolution: def test_bedrock_returns_client_with_credentials(self, monkeypatch): """With valid AWS credentials, Bedrock should return a usable client.""" - monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIAIOSFODNN7EXAMPLE") + monkeypatch.setenv("AWS_ACCESS_KEY_ID", "AKIAIO...MPLE") monkeypatch.setenv("AWS_SECRET_ACCESS_KEY", "wJalrXUtnFEMI/K7MDENG/bPxRfiCYEXAMPLEKEY") monkeypatch.setenv("AWS_REGION", "us-west-2") + # Isolate the pure-IAM path: a bearer token in the ambient environment + # (this repo's own dev setup uses one) would otherwise correctly route + # Claude through the Converse shim instead of the AnthropicBedrock SDK. + monkeypatch.delenv("AWS_BEARER_TOKEN_BEDROCK", raising=False) mock_anthropic_bedrock = MagicMock() with patch("agent.anthropic_adapter.build_anthropic_bedrock_client", diff --git a/tests/hermes_cli/test_runtime_provider_resolution.py b/tests/hermes_cli/test_runtime_provider_resolution.py index 3c12ba8a66ba..4b629760b974 100644 --- a/tests/hermes_cli/test_runtime_provider_resolution.py +++ b/tests/hermes_cli/test_runtime_provider_resolution.py @@ -1324,6 +1324,12 @@ def _patch_bedrock(monkeypatch, config_default=""): monkeypatch.setattr(ba, "has_aws_credentials", lambda: True) monkeypatch.setattr(ba, "resolve_aws_auth_env_var", lambda: "AWS_PROFILE") monkeypatch.setattr(ba, "resolve_bedrock_region", lambda: "eu-north-1") + # resolve_runtime_provider reads AWS_BEARER_TOKEN_BEDROCK from os.environ + # DIRECTLY (not via the mocked resolve_aws_auth_env_var), so an ambient + # bearer token — present in this repo's own dev environment — would force + # the Converse path and break the IAM/SSO dual-path assertions below. + # Delete it here so the helper models a pure IAM/SSO setup. + monkeypatch.delenv("AWS_BEARER_TOKEN_BEDROCK", raising=False) def test_resolve_runtime_provider_bedrock_claude_target_model_uses_anthropic_messages(monkeypatch):