diff --git a/python/packages/anthropic/agent_framework_anthropic/_bedrock_client.py b/python/packages/anthropic/agent_framework_anthropic/_bedrock_client.py index a63e3012bf0..215382b33a8 100644 --- a/python/packages/anthropic/agent_framework_anthropic/_bedrock_client.py +++ b/python/packages/anthropic/agent_framework_anthropic/_bedrock_client.py @@ -40,11 +40,11 @@ def __init__( self, *, model: str | None = None, - aws_secret_key: str | None = None, - aws_access_key: str | None = None, + aws_secret_key: str | SecretString | None = None, + aws_access_key: str | SecretString | None = None, aws_region: str | None = None, aws_profile: str | None = None, - aws_session_token: str | None = None, + aws_session_token: str | SecretString | None = None, base_url: str | None = None, anthropic_client: AsyncAnthropicBedrock | None = None, additional_beta_flags: list[str] | None = None, @@ -118,11 +118,11 @@ def __init__( self, *, model: str | None = None, - aws_secret_key: str | None = None, - aws_access_key: str | None = None, + aws_secret_key: str | SecretString | None = None, + aws_access_key: str | SecretString | None = None, aws_region: str | None = None, aws_profile: str | None = None, - aws_session_token: str | None = None, + aws_session_token: str | SecretString | None = None, base_url: str | None = None, anthropic_client: AsyncAnthropicBedrock | None = None, additional_beta_flags: list[str] | None = None, diff --git a/python/packages/anthropic/agent_framework_anthropic/_chat_client.py b/python/packages/anthropic/agent_framework_anthropic/_chat_client.py index f3fab4ca26f..82a25e874e2 100644 --- a/python/packages/anthropic/agent_framework_anthropic/_chat_client.py +++ b/python/packages/anthropic/agent_framework_anthropic/_chat_client.py @@ -270,7 +270,7 @@ class RawAnthropicClient( def __init__( self, *, - api_key: str | None = None, + api_key: str | SecretString | None = None, model: str | None = None, base_url: str | None = None, anthropic_client: AnthropicAsyncClient | None = None, @@ -1634,7 +1634,7 @@ class AnthropicClient( def __init__( self, *, - api_key: str | None = None, + api_key: str | SecretString | None = None, model: str | None = None, base_url: str | None = None, anthropic_client: AnthropicAsyncClient | None = None, diff --git a/python/packages/anthropic/agent_framework_anthropic/_foundry_client.py b/python/packages/anthropic/agent_framework_anthropic/_foundry_client.py index 2a9db1177ff..2b1ee0cd405 100644 --- a/python/packages/anthropic/agent_framework_anthropic/_foundry_client.py +++ b/python/packages/anthropic/agent_framework_anthropic/_foundry_client.py @@ -40,7 +40,7 @@ def __init__( *, model: str | None = None, resource: str | None = None, - api_key: str | None = None, + api_key: str | SecretString | None = None, azure_ad_token_provider: AnthropicFoundryAzureADTokenProvider | None = None, base_url: str | None = None, anthropic_client: AsyncAnthropicFoundry | None = None, @@ -123,7 +123,7 @@ def __init__( *, model: str | None = None, resource: str | None = None, - api_key: str | None = None, + api_key: str | SecretString | None = None, azure_ad_token_provider: AnthropicFoundryAzureADTokenProvider | None = None, base_url: str | None = None, anthropic_client: AsyncAnthropicFoundry | None = None, diff --git a/python/packages/anthropic/tests/test_anthropic_client.py b/python/packages/anthropic/tests/test_anthropic_client.py index d0b9a4a8f1a..468c6ca525a 100644 --- a/python/packages/anthropic/tests/test_anthropic_client.py +++ b/python/packages/anthropic/tests/test_anthropic_client.py @@ -21,7 +21,7 @@ SupportsChatGetResponse, tool, ) -from agent_framework._settings import load_settings +from agent_framework._settings import SecretString, load_settings from agent_framework._tools import SHELL_TOOL_KIND_VALUE from agent_framework.observability import ChatTelemetryLayer from anthropic.types.beta import ( @@ -94,17 +94,20 @@ def test_anthropic_settings_init(anthropic_unit_test_env: dict[str, str]) -> Non assert settings["chat_model"] == anthropic_unit_test_env["ANTHROPIC_CHAT_MODEL"] -def test_anthropic_settings_init_with_explicit_values() -> None: +@pytest.mark.parametrize("api_key", ["custom-api-key", SecretString("custom-api-key")], ids=["str", "secret"]) +def test_anthropic_settings_init_with_explicit_values(api_key: str | SecretString) -> None: """Test AnthropicSettings initialization with explicit values.""" settings = load_settings( AnthropicSettings, env_prefix="ANTHROPIC_", - api_key="custom-api-key", + api_key=api_key, chat_model="claude-3-opus-20240229", ) - assert settings["api_key"] is not None + assert isinstance(settings["api_key"], SecretString) assert settings["api_key"].get_secret_value() == "custom-api-key" + assert "custom-api-key" not in str(settings["api_key"]) + assert "custom-api-key" not in repr(settings) assert settings["chat_model"] == "claude-3-opus-20240229" @@ -153,16 +156,20 @@ def test_agent_accepts_anthropic_clients() -> None: assert agent.client is client +@pytest.mark.parametrize("secret_type", [str, SecretString], ids=["str", "secret"]) def test_anthropic_client_init_auto_create_client( anthropic_unit_test_env: dict[str, str], + secret_type: type[str] | type[SecretString], ) -> None: """Test AnthropicClient initialization with auto-created anthropic_client.""" client = AnthropicClient( - api_key=anthropic_unit_test_env["ANTHROPIC_API_KEY"], + api_key=secret_type(anthropic_unit_test_env["ANTHROPIC_API_KEY"]), model=anthropic_unit_test_env["ANTHROPIC_CHAT_MODEL"], ) assert client.anthropic_client is not None + assert type(client.anthropic_client.api_key) is str + assert client.anthropic_client.api_key == anthropic_unit_test_env["ANTHROPIC_API_KEY"] assert client.model == anthropic_unit_test_env["ANTHROPIC_CHAT_MODEL"] diff --git a/python/packages/anthropic/tests/test_anthropic_provider_clients.py b/python/packages/anthropic/tests/test_anthropic_provider_clients.py index 9dce241ea84..580acaff552 100644 --- a/python/packages/anthropic/tests/test_anthropic_provider_clients.py +++ b/python/packages/anthropic/tests/test_anthropic_provider_clients.py @@ -4,6 +4,7 @@ import pytest from agent_framework import Agent, ChatMiddlewareLayer, FunctionInvocationLayer +from agent_framework._settings import SecretString from agent_framework._telemetry import get_user_agent from agent_framework.observability import ChatTelemetryLayer @@ -107,7 +108,10 @@ def test_agent_accepts_anthropic_vertex_clients() -> None: assert agent.client is client -def test_raw_anthropic_foundry_client_creates_sdk_client_from_settings(tmp_path) -> None: +@pytest.mark.parametrize("api_key", [None, "test-key", SecretString("test-key")], ids=["env", "str", "secret"]) +def test_raw_anthropic_foundry_client_creates_sdk_client_from_settings( + tmp_path, api_key: str | SecretString | None +) -> None: env_file = tmp_path / ".env" env_file.write_text( "ANTHROPIC_CHAT_MODEL=claude-foundry-test\n" @@ -119,10 +123,11 @@ def test_raw_anthropic_foundry_client_creates_sdk_client_from_settings(tmp_path) with patch( "agent_framework_anthropic._foundry_client.AsyncAnthropicFoundry", return_value=mock_transport ) as factory: - client = RawAnthropicFoundryClient(env_file_path=str(env_file)) + client = RawAnthropicFoundryClient(api_key=api_key, env_file_path=str(env_file)) assert client.model == "claude-foundry-test" assert client.anthropic_client is mock_transport + assert type(factory.call_args.kwargs["api_key"]) is str factory.assert_called_once_with( resource="test-resource", api_key="test-key", @@ -174,7 +179,10 @@ def test_raw_anthropic_foundry_client_requires_resource_or_base_url() -> None: RawAnthropicFoundryClient() -def test_raw_anthropic_bedrock_client_creates_sdk_client_from_arguments() -> None: +@pytest.mark.parametrize("secret_type", [str, SecretString], ids=["str", "secret"]) +def test_raw_anthropic_bedrock_client_creates_sdk_client_from_arguments( + secret_type: type[str] | type[SecretString], +) -> None: mock_transport = _create_mock_transport("https://bedrock-runtime.us-east-1.amazonaws.com") with patch( @@ -182,8 +190,9 @@ def test_raw_anthropic_bedrock_client_creates_sdk_client_from_arguments() -> Non ) as factory: client = RawAnthropicBedrockClient( model="claude-bedrock-test", - aws_access_key="access-key", - aws_secret_key="secret-key", + aws_access_key=secret_type("access-key"), + aws_secret_key=secret_type("secret-key"), + aws_session_token=secret_type("session-token"), aws_region="us-east-1", ) @@ -194,10 +203,12 @@ def test_raw_anthropic_bedrock_client_creates_sdk_client_from_arguments() -> Non aws_access_key="access-key", aws_region="us-east-1", aws_profile=None, - aws_session_token=None, + aws_session_token="session-token", base_url=None, default_headers={"User-Agent": get_user_agent()}, ) + for key in ("aws_access_key", "aws_secret_key", "aws_session_token"): + assert type(factory.call_args.kwargs[key]) is str def test_raw_anthropic_vertex_client_creates_sdk_client_from_arguments() -> None: diff --git a/python/packages/azure-ai-search/agent_framework_azure_ai_search/_context_provider.py b/python/packages/azure-ai-search/agent_framework_azure_ai_search/_context_provider.py index faabd6d7bdd..3bcd83c6619 100644 --- a/python/packages/azure-ai-search/agent_framework_azure_ai_search/_context_provider.py +++ b/python/packages/azure-ai-search/agent_framework_azure_ai_search/_context_provider.py @@ -188,7 +188,7 @@ def __init__( source_id: str = DEFAULT_SOURCE_ID, endpoint: str | None = None, index_name: str | None = None, - api_key: str | AzureKeyCredential | None = None, + api_key: str | SecretString | AzureKeyCredential | None = None, credential: AzureCredentialTypes | None = None, *, mode: Literal["semantic"] = "semantic", @@ -201,7 +201,7 @@ def __init__( model: str | None = None, knowledge_base_name: None = None, retrieval_instructions: str | None = None, - azure_openai_api_key: str | None = None, + azure_openai_api_key: str | SecretString | None = None, knowledge_base_output_mode: KnowledgeBaseOutputModeLiteral = "extractive_data", retrieval_reasoning_effort: RetrievalReasoningEffortLiteral = "minimal", query_source_credential: AzureCredentialTypes | None = None, @@ -243,7 +243,7 @@ def __init__( source_id: str = DEFAULT_SOURCE_ID, endpoint: str | None = None, index_name: str | None = None, - api_key: str | AzureKeyCredential | None = None, + api_key: str | SecretString | AzureKeyCredential | None = None, credential: AzureCredentialTypes | None = None, *, mode: Literal["agentic"], @@ -256,7 +256,7 @@ def __init__( model: str, knowledge_base_name: None = None, retrieval_instructions: str | None = None, - azure_openai_api_key: str | None = None, + azure_openai_api_key: str | SecretString | None = None, knowledge_base_output_mode: KnowledgeBaseOutputModeLiteral = "extractive_data", retrieval_reasoning_effort: RetrievalReasoningEffortLiteral = "minimal", query_source_credential: AzureCredentialTypes | None = None, @@ -299,7 +299,7 @@ def __init__( source_id: str = DEFAULT_SOURCE_ID, endpoint: str | None = None, index_name: None = None, - api_key: str | AzureKeyCredential | None = None, + api_key: str | SecretString | AzureKeyCredential | None = None, credential: AzureCredentialTypes | None = None, *, mode: Literal["agentic"], @@ -312,7 +312,7 @@ def __init__( model: str | None = None, knowledge_base_name: str, retrieval_instructions: str | None = None, - azure_openai_api_key: str | None = None, + azure_openai_api_key: str | SecretString | None = None, knowledge_base_output_mode: KnowledgeBaseOutputModeLiteral = "extractive_data", retrieval_reasoning_effort: RetrievalReasoningEffortLiteral = "minimal", query_source_credential: AzureCredentialTypes | None = None, @@ -355,7 +355,7 @@ def __init__( source_id: str = DEFAULT_SOURCE_ID, endpoint: str | None = None, index_name: None = None, - api_key: str | AzureKeyCredential | None = None, + api_key: str | SecretString | AzureKeyCredential | None = None, credential: AzureCredentialTypes | None = None, *, mode: Literal["agentic"], @@ -368,7 +368,7 @@ def __init__( model: str | None = None, knowledge_base_name: None = None, retrieval_instructions: str | None = None, - azure_openai_api_key: str | None = None, + azure_openai_api_key: str | SecretString | None = None, knowledge_base_output_mode: KnowledgeBaseOutputModeLiteral = "extractive_data", retrieval_reasoning_effort: RetrievalReasoningEffortLiteral = "minimal", query_source_credential: AzureCredentialTypes | None = None, @@ -414,7 +414,7 @@ def __init__( source_id: str = DEFAULT_SOURCE_ID, endpoint: str | None = None, index_name: str | None = None, - api_key: str | AzureKeyCredential | None = None, + api_key: str | SecretString | AzureKeyCredential | None = None, credential: AzureCredentialTypes | None = None, *, mode: Literal["semantic", "agentic"] = "semantic", @@ -427,7 +427,7 @@ def __init__( model: str | None = None, knowledge_base_name: str | None = None, retrieval_instructions: str | None = None, - azure_openai_api_key: str | None = None, + azure_openai_api_key: str | SecretString | None = None, knowledge_base_output_mode: KnowledgeBaseOutputModeLiteral = "extractive_data", retrieval_reasoning_effort: RetrievalReasoningEffortLiteral = "minimal", query_source_credential: AzureCredentialTypes | None = None, @@ -499,7 +499,7 @@ def __init__( endpoint=endpoint, index_name=index_name, knowledge_base_name=knowledge_base_name, - api_key=api_key if isinstance(api_key, str) else None, + api_key=api_key if isinstance(api_key, (str, SecretString)) else None, env_file_path=env_file_path, env_file_encoding=env_file_encoding, ) @@ -866,7 +866,9 @@ async def _ensure_knowledge_base(self) -> None: resource_url=self.azure_openai_resource_url, deployment_name=self.azure_openai_model, model_name=self.azure_openai_model, - api_key=self.azure_openai_api_key, + api_key=self.azure_openai_api_key.get_secret_value() + if isinstance(self.azure_openai_api_key, SecretString) + else self.azure_openai_api_key, ) kb_kwargs: dict[str, Any] = { diff --git a/python/packages/azure-ai-search/tests/test_aisearch_context_provider.py b/python/packages/azure-ai-search/tests/test_aisearch_context_provider.py index 3672b6e2016..776950974a2 100644 --- a/python/packages/azure-ai-search/tests/test_aisearch_context_provider.py +++ b/python/packages/azure-ai-search/tests/test_aisearch_context_provider.py @@ -12,6 +12,7 @@ import pytest from agent_framework import Content, Message from agent_framework._sessions import AgentSession, SessionContext +from agent_framework._settings import SecretString from agent_framework.exceptions import SettingNotFoundError from azure.core.credentials import AzureKeyCredential from azure.core.pipeline.transport import AioHttpTransport @@ -205,6 +206,9 @@ def test_env_variable_fallback(self) -> None: provider = AzureAISearchContextProvider(source_id="env-test") assert provider.endpoint == "https://env.search.windows.net" assert provider.index_name == "env-index" + assert isinstance(provider.credential, AzureKeyCredential) + assert type(provider.credential.key) is str + assert provider.credential.key == "env-key" def test_top_k_and_semantic_config(self) -> None: provider = _make_provider(top_k=10, semantic_configuration_name="my-config") @@ -235,6 +239,18 @@ def test_model_explicit(self) -> None: class TestInitCredentialResolution: """Tests for credential resolution paths.""" + @pytest.mark.parametrize("api_key", ["test-key", SecretString("test-key")], ids=["str", "secret"]) + def test_api_key_unwrapped(self, api_key: str | SecretString, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("AZURE_SEARCH_API_KEY", "env-key") + provider = AzureAISearchContextProvider( + endpoint="https://test.search.windows.net", + index_name="idx", + api_key=api_key, + ) + assert isinstance(provider.credential, AzureKeyCredential) + assert type(provider.credential.key) is str + assert provider.credential.key == "test-key" + def test_token_credential_used(self) -> None: mock_cred = AsyncMock() provider = AzureAISearchContextProvider( @@ -1273,10 +1289,11 @@ async def test_missing_index_name_raises(self) -> None: with pytest.raises(ValueError, match="index_name is required"): await provider._ensure_knowledge_base() - async def test_creates_knowledge_source_when_not_found(self) -> None: + @pytest.mark.parametrize("api_key", [None, "aoai-key", SecretString("aoai-key")], ids=["none", "str", "secret"]) + async def test_creates_knowledge_source_when_not_found(self, api_key: str | SecretString | None) -> None: from azure.core.exceptions import ResourceNotFoundError - provider = _make_provider() + provider = _make_provider(azure_openai_api_key=api_key) provider._knowledge_base_initialized = False provider._use_existing_knowledge_base = False provider.knowledge_base_name = "test-kb" @@ -1296,6 +1313,16 @@ async def test_creates_knowledge_source_when_not_found(self) -> None: mock_index_client.create_knowledge_source.assert_awaited_once() mock_index_client.create_or_update_knowledge_base.assert_awaited_once() + knowledge_base = mock_index_client.create_or_update_knowledge_base.call_args.args[0] + sdk_api_key = knowledge_base.models[0].azure_open_ai_parameters.api_key + if api_key is None: + assert sdk_api_key is None + else: + assert type(sdk_api_key) is str + assert sdk_api_key == "aoai-key" + if isinstance(api_key, SecretString): + assert provider.azure_openai_api_key is api_key + assert "aoai-key" not in repr(provider.azure_openai_api_key) assert provider._knowledge_base_initialized is True async def test_uses_existing_knowledge_source(self) -> None: diff --git a/python/packages/azure-cosmos/agent_framework_azure_cosmos/_checkpoint_storage.py b/python/packages/azure-cosmos/agent_framework_azure_cosmos/_checkpoint_storage.py index e441975600d..3ddd0e0c73f 100644 --- a/python/packages/azure-cosmos/agent_framework_azure_cosmos/_checkpoint_storage.py +++ b/python/packages/azure-cosmos/agent_framework_azure_cosmos/_checkpoint_storage.py @@ -120,7 +120,7 @@ def __init__( endpoint: str | None = None, database_name: str | None = None, container_name: str | None = None, - credential: str | AzureCredentialTypes | None = None, + credential: str | SecretString | AzureCredentialTypes | None = None, cosmos_client: CosmosClient | None = None, container_client: ContainerProxy | None = None, env_file_path: str | None = None, @@ -186,7 +186,7 @@ def __init__( endpoint=endpoint, database_name=database_name, container_name=container_name, - key=credential if isinstance(credential, str) else None, + key=credential if isinstance(credential, (str, SecretString)) else None, env_file_path=env_file_path, env_file_encoding=env_file_encoding, ) @@ -194,6 +194,8 @@ def __init__( self.container_name = settings["container_name"] # type: ignore[assignment] if self._cosmos_client is None: + if isinstance(credential, SecretString): + credential = credential.get_secret_value() self._cosmos_client = CosmosClient( url=settings["endpoint"], # type: ignore[arg-type] credential=credential or settings["key"].get_secret_value(), # type: ignore[arg-type,union-attr] diff --git a/python/packages/azure-cosmos/agent_framework_azure_cosmos/_history_provider.py b/python/packages/azure-cosmos/agent_framework_azure_cosmos/_history_provider.py index 507fe228b05..9d9cb2968d5 100644 --- a/python/packages/azure-cosmos/agent_framework_azure_cosmos/_history_provider.py +++ b/python/packages/azure-cosmos/agent_framework_azure_cosmos/_history_provider.py @@ -53,7 +53,7 @@ def __init__( endpoint: str | None = None, database_name: str | None = None, container_name: str | None = None, - credential: str | AzureCredentialTypes | None = None, + credential: str | SecretString | AzureCredentialTypes | None = None, cosmos_client: CosmosClient | None = None, container_client: ContainerProxy | None = None, env_file_path: str | None = None, @@ -114,13 +114,15 @@ def __init__( endpoint=endpoint, database_name=database_name, container_name=container_name, - key=credential if isinstance(credential, str) else None, + key=credential if isinstance(credential, (str, SecretString)) else None, env_file_path=env_file_path, env_file_encoding=env_file_encoding, ) self.database_name = settings["database_name"] # type: ignore[assignment] self.container_name = settings["container_name"] # type: ignore[assignment] if self._cosmos_client is None: + if isinstance(credential, SecretString): + credential = credential.get_secret_value() self._cosmos_client = CosmosClient( url=settings["endpoint"], # type: ignore[arg-type] credential=credential or settings["key"].get_secret_value(), # type: ignore[arg-type,union-attr] diff --git a/python/packages/azure-cosmos/tests/test_cosmos_checkpoint_storage.py b/python/packages/azure-cosmos/tests/test_cosmos_checkpoint_storage.py index 0fe47d5a5da..e4fd305171c 100644 --- a/python/packages/azure-cosmos/tests/test_cosmos_checkpoint_storage.py +++ b/python/packages/azure-cosmos/tests/test_cosmos_checkpoint_storage.py @@ -11,6 +11,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest +from agent_framework._settings import SecretString from agent_framework._workflows._checkpoint import WorkflowCheckpoint from agent_framework._workflows._checkpoint_encoding import encode_checkpoint_value from agent_framework.exceptions import SettingNotFoundError, WorkflowCheckpointException @@ -129,17 +130,18 @@ async def test_init_missing_required_settings_raises(monkeypatch: pytest.MonkeyP CosmosCheckpointStorage() +@pytest.mark.parametrize( + "credential", [None, "key-123", SecretString("key-123"), MagicMock()], ids=["env", "str", "secret", "token"] +) async def test_init_constructs_client_with_credential( - monkeypatch: pytest.MonkeyPatch, mock_cosmos_client: MagicMock + monkeypatch: pytest.MonkeyPatch, + mock_cosmos_client: MagicMock, + credential: str | SecretString | MagicMock | None, ) -> None: - """Uses key-based auth when a key string is provided, otherwise falls back to Azure credential (RBAC).""" + """Unwrap explicit and environment keys while preserving Azure token credentials.""" mock_factory = MagicMock(return_value=mock_cosmos_client) monkeypatch.setattr(checkpoint_storage_module, "CosmosClient", mock_factory) - monkeypatch.delenv("AZURE_COSMOS_KEY", raising=False) - - # Simulate real-world pattern: use key if available, else RBAC credential - cosmos_key = os.getenv("AZURE_COSMOS_KEY") - credential: Any = cosmos_key if cosmos_key else MagicMock() # MagicMock simulates DefaultAzureCredential() + monkeypatch.setenv("AZURE_COSMOS_KEY", "env-key") CosmosCheckpointStorage( endpoint="https://account.documents.azure.com:443/", @@ -151,7 +153,11 @@ async def test_init_constructs_client_with_credential( mock_factory.assert_called_once() kwargs = mock_factory.call_args.kwargs assert kwargs["url"] == "https://account.documents.azure.com:443/" - assert kwargs["credential"] is credential + if isinstance(credential, MagicMock): + assert kwargs["credential"] is credential + else: + assert type(kwargs["credential"]) is str + assert kwargs["credential"] == ("env-key" if credential is None else "key-123") async def test_init_creates_database_and_container(mock_cosmos_client: MagicMock) -> None: diff --git a/python/packages/azure-cosmos/tests/test_cosmos_history_provider.py b/python/packages/azure-cosmos/tests/test_cosmos_history_provider.py index 7e581a7636e..e2c24016899 100644 --- a/python/packages/azure-cosmos/tests/test_cosmos_history_provider.py +++ b/python/packages/azure-cosmos/tests/test_cosmos_history_provider.py @@ -12,6 +12,7 @@ import pytest from agent_framework import AgentResponse, Message from agent_framework._sessions import AgentSession, SessionContext +from agent_framework._settings import SecretString from agent_framework.exceptions import SettingNotFoundError from azure.cosmos.aio import CosmosClient from azure.cosmos.exceptions import CosmosResourceNotFoundError @@ -104,15 +105,22 @@ def test_missing_required_settings_raises(self, monkeypatch: pytest.MonkeyPatch) with pytest.raises(SettingNotFoundError, match="database_name"): CosmosHistoryProvider() - def test_constructs_client_with_string_credential( - self, monkeypatch: pytest.MonkeyPatch, mock_cosmos_client: MagicMock + @pytest.mark.parametrize( + "credential", [None, "key-123", SecretString("key-123"), MagicMock()], ids=["env", "str", "secret", "token"] + ) + def test_constructs_client_with_credential( + self, + monkeypatch: pytest.MonkeyPatch, + mock_cosmos_client: MagicMock, + credential: str | SecretString | MagicMock | None, ) -> None: mock_factory = MagicMock(return_value=mock_cosmos_client) monkeypatch.setattr(history_provider_module, "CosmosClient", mock_factory) + monkeypatch.setenv("AZURE_COSMOS_KEY", "env-key") CosmosHistoryProvider( endpoint="https://account.documents.azure.com:443/", - credential="key-123", + credential=credential, database_name="db1", container_name="history", ) @@ -120,7 +128,11 @@ def test_constructs_client_with_string_credential( mock_factory.assert_called_once() kwargs = mock_factory.call_args.kwargs assert kwargs["url"] == "https://account.documents.azure.com:443/" - assert kwargs["credential"] == "key-123" + if isinstance(credential, MagicMock): + assert kwargs["credential"] is credential + else: + assert type(kwargs["credential"]) is str + assert kwargs["credential"] == ("env-key" if credential is None else "key-123") class TestCosmosHistoryProviderContainerConfig: diff --git a/python/packages/bedrock/agent_framework_bedrock/_chat_client.py b/python/packages/bedrock/agent_framework_bedrock/_chat_client.py index c38b813fdae..d719027db0a 100644 --- a/python/packages/bedrock/agent_framework_bedrock/_chat_client.py +++ b/python/packages/bedrock/agent_framework_bedrock/_chat_client.py @@ -240,9 +240,9 @@ def __init__( *, region: str | None = None, model: str | None = None, - access_key: str | None = None, - secret_key: str | None = None, - session_token: str | None = None, + access_key: str | SecretString | None = None, + secret_key: str | SecretString | None = None, + session_token: str | SecretString | None = None, client: BaseClient | None = None, boto3_session: Boto3Session | None = None, additional_properties: dict[str, Any] | None = None, diff --git a/python/packages/bedrock/agent_framework_bedrock/_embedding_client.py b/python/packages/bedrock/agent_framework_bedrock/_embedding_client.py index cd71a9d0eb0..9d55d4a22f8 100644 --- a/python/packages/bedrock/agent_framework_bedrock/_embedding_client.py +++ b/python/packages/bedrock/agent_framework_bedrock/_embedding_client.py @@ -101,9 +101,9 @@ def __init__( *, region: str | None = None, model: str | None = None, - access_key: str | None = None, - secret_key: str | None = None, - session_token: str | None = None, + access_key: str | SecretString | None = None, + secret_key: str | SecretString | None = None, + session_token: str | SecretString | None = None, client: BaseClient | None = None, boto3_session: Boto3Session | None = None, additional_properties: dict[str, Any] | None = None, @@ -271,9 +271,9 @@ def __init__( *, region: str | None = None, model: str | None = None, - access_key: str | None = None, - secret_key: str | None = None, - session_token: str | None = None, + access_key: str | SecretString | None = None, + secret_key: str | SecretString | None = None, + session_token: str | SecretString | None = None, client: BaseClient | None = None, boto3_session: Boto3Session | None = None, otel_provider_name: str | None = None, diff --git a/python/packages/bedrock/tests/test_bedrock_client.py b/python/packages/bedrock/tests/test_bedrock_client.py index 6d339ae3c59..41e277d75f3 100644 --- a/python/packages/bedrock/tests/test_bedrock_client.py +++ b/python/packages/bedrock/tests/test_bedrock_client.py @@ -14,7 +14,7 @@ from boto3.session import Session as Boto3Session from botocore.client import BaseClient -from agent_framework_bedrock import BedrockChatClient +from agent_framework_bedrock import BedrockChatClient, BedrockEmbeddingClient from agent_framework_bedrock._chat_client import BedrockSettings from agent_framework_bedrock._feature_usage import FeatureIndex @@ -276,6 +276,38 @@ def client(self, service_name: str, *, region_name: str, config: Any) -> _StubBe ] +@pytest.mark.parametrize("secret_type", [str, SecretString], ids=["str", "secret"]) +@pytest.mark.parametrize( + ("client_type", "module"), + [(BedrockChatClient, "_chat_client"), (BedrockEmbeddingClient, "_embedding_client")], + ids=["chat", "embedding"], +) +def test_constructor_unwraps_session_credentials( + secret_type: type[str] | type[SecretString], + client_type: type[BedrockChatClient] | type[BedrockEmbeddingClient], + module: str, +) -> None: + session = MagicMock(region_name="eu-west-1") + session.client.return_value = _StubBedrockRuntime() + with patch(f"agent_framework_bedrock.{module}.Boto3Session", return_value=session) as session_cls: + client = client_type( + model="test-model", + region="eu-west-1", + access_key=secret_type("access"), + secret_key=secret_type("secret"), + session_token=secret_type("token"), + ) + + assert client.model == "test-model" + session_cls.assert_called_once_with( + region_name="eu-west-1", + aws_access_key_id="access", + aws_secret_access_key="secret", + aws_session_token="token", + ) + assert all(type(value) is str for value in session_cls.call_args.kwargs.values()) + + def test_create_session_uses_secret_values() -> None: """Bedrock session creation should unwrap configured secret values.""" settings: BedrockSettings = { @@ -294,6 +326,9 @@ def test_create_session_uses_secret_values() -> None: aws_secret_access_key="secret", aws_session_token="token", ) + assert all(type(value) is str for value in session_cls.call_args.kwargs.values()) + assert isinstance(settings["secret_key"], SecretString) + assert str(settings["secret_key"]) == "**********" def test_invoke_converse_requires_mapping_response() -> None: diff --git a/python/packages/core/AGENTS.md b/python/packages/core/AGENTS.md index 257782f7c34..7fe230f2adc 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -37,6 +37,14 @@ agent_framework/ - Public deprecation behavior for a lazy export belongs in the owning module. The root package should delegate via the normal lazy export map instead of carrying one-off branches. +### Settings (`_settings.py`) + +- **`SecretString`** is a non-`str` wrapper: string conversion, representation, formatting, and concatenation + display a mask. String-only APIs such as `str.join()` and JSON encoding reject it unless callers explicitly + convert it. Use `get_secret_value()` when passing credentials to provider SDKs; `str(secret)` returns the mask. +- **`load_settings`** accepts plain string overrides for `SecretString` fields and wraps them, as it does for + environment and `.env` values. Existing `SecretString` overrides are preserved. + ### Agents (`_agents.py`) - **`SupportsAgentRun`** - Protocol defining the agent interface diff --git a/python/packages/core/agent_framework/_settings.py b/python/packages/core/agent_framework/_settings.py index 60b8e31353a..bcd6828d892 100644 --- a/python/packages/core/agent_framework/_settings.py +++ b/python/packages/core/agent_framework/_settings.py @@ -49,37 +49,85 @@ class MySettings(TypedDict, total=False): SettingsT = TypeVar("SettingsT", default=dict[str, Any]) -class SecretString(str): - """A string subclass that masks its value in repr() to prevent accidental exposure. +class SecretString: + """A secret value that masks string conversion to prevent accidental exposure. - SecretString behaves exactly like a regular string in all operations, - but its repr() shows '**********' instead of the actual value. - This helps prevent secrets from being accidentally logged or displayed. + ``str()``, ``repr()``, formatting, and concatenation display '**********' + instead of the actual value. Equality, hashing, length, and truthiness + operate on the underlying value. - It also provides a ``get_secret_value()`` method for backward compatibility - with code that previously used ``pydantic.SecretStr``. + This is not a ``str`` subclass. Use ``get_secret_value()`` when passing a + credential to an SDK or another API that requires a real string. String-only + operations such as ``str.join()`` and JSON encoding reject the wrapper + rather than implicitly exposing its value. Explicitly extracted secrets + are ordinary strings and are no longer protected from accidental exposure. + + Args: + value: The secret string or an existing ``SecretString`` to wrap. Example: ```python api_key = SecretString("sk-secret-key") - print(api_key) # sk-secret-key (normal string behavior) + print(api_key) # ********** print(repr(api_key)) # SecretString('**********') - print(f"Key: {api_key}") # Key: sk-secret-key + print(f"Key: {api_key}") # Key: ********** + print("Bearer " + api_key) # Bearer ********** print(api_key.get_secret_value()) # sk-secret-key ``` """ + __slots__ = ("_value",) + + def __init__(self, value: str | SecretString) -> None: + if isinstance(value, SecretString): + value = value.get_secret_value() + if not isinstance(value, str): + raise TypeError("SecretString requires a string value.") + self._value = value + + def __str__(self) -> str: + """Return a masked string to prevent secret exposure.""" + return "**********" + def __repr__(self) -> str: """Return a masked representation to prevent secret exposure.""" return "SecretString('**********')" - def get_secret_value(self) -> str: - """Return the underlying string value. + def __format__(self, format_spec: str) -> str: + """Apply string formatting to the masked value.""" + return format(str(self), format_spec) + + def __add__(self, other: str | SecretString) -> str: + """Concatenate strings without exposing secret values.""" + if isinstance(other, (str, SecretString)): + return str(self) + str(other) + return NotImplemented + + def __radd__(self, other: str | SecretString) -> str: + """Concatenate strings without exposing secret values.""" + if isinstance(other, (str, SecretString)): + return str(other) + str(self) + return NotImplemented + + def __eq__(self, other: object) -> bool: + """Compare the underlying values.""" + if isinstance(other, SecretString): + return self._value == other._value + if isinstance(other, str): + return self._value == other + return NotImplemented + + def __hash__(self) -> int: + """Hash the underlying value.""" + return hash(self._value) + + def __len__(self) -> int: + """Return the length of the underlying value.""" + return len(self._value) - Provided for backward compatibility with ``pydantic.SecretStr``. - Since SecretString *is* a str, this simply returns ``str(self)``. - """ - return str(self) + def get_secret_value(self) -> str: + """Explicitly return the unmasked string value for credential use.""" + return self._value def _coerce_value(value: str, target_type: type) -> Any: @@ -175,7 +223,7 @@ def _check_override_type(value: Any, field_type: type, field_name: str) -> None: if not isinstance(value, allowed): # Allow str for SecretString fields (will be coerced) - if isinstance(value, str) and any(isinstance(a, type) and issubclass(a, str) for a in allowed): + if isinstance(value, str) and any(issubclass(a, (str, SecretString)) for a in allowed): return # Allow int for float fields (standard numeric promotion) if isinstance(value, int) and float in allowed: @@ -256,7 +304,7 @@ def load_settings( override_value = overrides[field_name] _check_override_type(override_value, field_type, field_name) # Coerce plain str → SecretString if the annotation expects it - if isinstance(override_value, str) and not isinstance(override_value, SecretString): + if isinstance(override_value, str): with suppress(ValueError, TypeError): coerced = _coerce_value(override_value, field_type) if isinstance(coerced, SecretString): diff --git a/python/packages/core/tests/core/test_settings.py b/python/packages/core/tests/core/test_settings.py index 58d21a397c7..fb810c9188c 100644 --- a/python/packages/core/tests/core/test_settings.py +++ b/python/packages/core/tests/core/test_settings.py @@ -2,8 +2,12 @@ """Tests for load_settings() function.""" +import json +import logging import os import tempfile +from collections.abc import Callable +from pathlib import Path from typing import Any, Literal, TypedDict import pytest @@ -164,16 +168,151 @@ def test_secretstring_from_override(self) -> None: assert isinstance(settings["api_key"], SecretString) assert settings["api_key"] == "kwarg-secret" + def test_secretstring_from_wrapped_override(self) -> None: + secret = SecretString("my-secret") + + settings = load_settings(SecretSettings, env_prefix="SECRET_", api_key=secret) + + assert settings["api_key"] is secret + assert settings["api_key"].get_secret_value() == "my-secret" + + def test_secretstring_from_dotenv(self, tmp_path: Path) -> None: + env_file = tmp_path / ".env" + env_file.write_text("SECRET_API_KEY=my-secret\n", encoding="utf-8") + + settings = load_settings(SecretSettings, env_prefix="SECRET_", env_file_path=str(env_file)) + + assert isinstance(settings["api_key"], SecretString) + assert settings["api_key"].get_secret_value() == "my-secret" + assert str(settings["api_key"]) == "**********" + def test_secretstring_masked_in_repr(self) -> None: s = SecretString("my-secret") assert "my-secret" not in repr(s) assert "**********" in repr(s) + @pytest.mark.parametrize("value", ["my-secret", "", "x", "secret\nwith\tcontrols"]) + @pytest.mark.parametrize( + "render", + [ + pytest.param(str, id="str"), + pytest.param(lambda secret: f"{secret}", id="f-string"), + pytest.param(lambda secret: f"{secret!s}", id="f-string-str"), + pytest.param(lambda secret, template="%s": template % secret, id="percent"), + pytest.param(lambda secret, template="%(key)s": template % {"key": secret}, id="percent-mapping"), + pytest.param("{}".format, id="format"), + pytest.param(lambda secret: "{key}".format_map({"key": secret}), id="format-map"), + ], + ) + def test_secretstring_masked_in_string_conversion(self, value: str, render: Callable[[SecretString], str]) -> None: + assert render(SecretString(value)) == "**********" + + @pytest.mark.parametrize("format_spec", ["", "s", ">20", "*^20", ".3", "20.3"]) + def test_secretstring_format_spec_applies_to_mask(self, format_spec: str) -> None: + secret = SecretString("my-secret") + + assert format(secret, format_spec) == format("**********", format_spec) + assert f"{secret:{format_spec}}" == format("**********", format_spec) + assert "{0:{1}}".format(secret, format_spec) == format("**********", format_spec) + + def test_secretstring_concatenation_masks_both_operands(self) -> None: + secret = SecretString("my-secret") + + assert "Bearer " + secret == "Bearer **********" + assert secret + " suffix" == "********** suffix" + assert secret + SecretString("another-secret") == "********************" + assert "prefix " + secret + " suffix" == "prefix ********** suffix" + + def test_secretstring_masked_in_containers(self) -> None: + secret = SecretString("my-secret") + + assert str({"key": secret}) == "{'key': SecretString('**********')}" + assert repr([secret]) == "[SecretString('**********')]" + assert str({secret: "value"}) == "{SecretString('**********'): 'value'}" + + def test_secretstring_masked_in_print(self, capsys: pytest.CaptureFixture[str]) -> None: + print(SecretString("my-secret")) # noqa: T201 + + assert capsys.readouterr().out == "**********\n" + + def test_secretstring_masked_in_logging(self, caplog: pytest.LogCaptureFixture) -> None: + secret = SecretString("my-secret") + logger = logging.getLogger(__name__) + format_message = "Key: {}".format + + with caplog.at_level(logging.INFO, logger=__name__): + logger.info("Key: %s", secret) + logger.info("Key: %(key)s", {"key": secret}) + logger.info(f"Key: {secret}") + logger.info(format_message(secret)) + logger.info("Key: " + secret) + logger.info(secret) + + assert caplog.messages == ["Key: **********"] * 5 + ["**********"] + + def test_secretstring_masked_in_exception(self) -> None: + secret = SecretString("my-secret") + + assert str(ValueError(secret)) == "**********" + assert str(ValueError(f"Invalid key: {secret}")) == "Invalid key: **********" + assert "my-secret" not in repr(ValueError(secret)) + + @pytest.mark.parametrize( + "consume", + [ + pytest.param(lambda secret: "".join([secret]), id="join"), + pytest.param(lambda secret: json.dumps(secret), id="json-value"), + pytest.param(lambda secret: json.dumps({"key": secret}), id="json-container"), + pytest.param(lambda secret: json.dumps({secret: "value"}), id="json-key"), + pytest.param(lambda secret: str.__str__(secret), id="str-base-method"), + pytest.param(lambda secret: secret[:], id="slice"), + pytest.param(lambda secret: secret + 1, id="add-non-string"), + pytest.param(lambda secret: 1 + secret, id="radd-non-string"), + ], + ) + def test_secretstring_rejects_implicit_raw_string_use(self, consume: Callable[[Any], Any]) -> None: + secret = SecretString("my-secret") + + assert not isinstance(secret, str) + with pytest.raises(TypeError) as exc_info: + consume(secret) + assert "my-secret" not in str(exc_info.value) + + def test_secretstring_json_with_explicit_string_conversion_masks(self) -> None: + assert json.dumps({"key": SecretString("my-secret")}, default=str) == '{"key": "**********"}' + + @pytest.mark.parametrize("value", ["my-secret", ""]) + def test_secretstring_value_semantics(self, value: str) -> None: + secret = SecretString(value) + + assert len(secret) == len(value) + assert bool(secret) == bool(value) + assert secret == SecretString(value) + assert secret == value + assert value == secret + assert secret != SecretString("another-secret") + assert secret != "another-secret" + assert secret != object() + assert hash(secret) == hash(value) + assert {secret, SecretString(value), value} == {value} + + def test_secretstring_can_wrap_existing_secret(self) -> None: + secret = SecretString(SecretString("my-secret")) + + assert secret.get_secret_value() == "my-secret" + assert str(secret) == "**********" + + @pytest.mark.parametrize("value", [None, 123, b"my-secret"]) + def test_secretstring_rejects_non_strings(self, value: Any) -> None: + with pytest.raises(TypeError, match="^SecretString requires a string value\\.$"): + SecretString(value) + def test_get_secret_value_compat(self) -> None: s = SecretString("my-secret") assert s.get_secret_value() == "my-secret" - assert isinstance(s.get_secret_value(), str) + assert type(s.get_secret_value()) is str + assert f"Bearer {s.get_secret_value()}" == "Bearer my-secret" class TestTypeCoercion: @@ -261,6 +400,23 @@ def test_str_accepted_for_secretstring(self) -> None: assert isinstance(settings["api_key"], SecretString) assert settings["api_key"] == "plain-string" + def test_str_accepted_for_required_secretstring(self) -> None: + class RequiredSecretSettings(TypedDict): + api_key: SecretString + + settings = load_settings(RequiredSecretSettings, api_key="my-secret", required_fields=["api_key"]) + + assert isinstance(settings["api_key"], SecretString) + assert settings["api_key"].get_secret_value() == "my-secret" + + def test_invalid_secretstring_override_rejected(self) -> None: + with pytest.raises(ValueError, match="expected SecretString, got int"): + load_settings(SecretSettings, api_key=123) + + def test_secretstring_requires_explicit_unwrapping_for_str_field(self) -> None: + with pytest.raises(ValueError, match="expected str, got SecretString"): + load_settings(SimpleSettings, api_key=SecretString("my-secret")) + def test_parameterized_generic_union_arm_accepted(self) -> None: """A ``dict`` override is valid for ``dict[str, Any] | str | None``.""" diff --git a/python/packages/gemini/agent_framework_gemini/_chat_client.py b/python/packages/gemini/agent_framework_gemini/_chat_client.py index 28d54de8a4a..e611366a19d 100644 --- a/python/packages/gemini/agent_framework_gemini/_chat_client.py +++ b/python/packages/gemini/agent_framework_gemini/_chat_client.py @@ -320,7 +320,7 @@ class RawGeminiChatClient( def __init__( self, *, - api_key: str | None = None, + api_key: str | SecretString | None = None, model: str | None = None, vertexai: bool | None = None, project: str | None = None, @@ -1351,7 +1351,7 @@ class GeminiChatClient( def __init__( self, *, - api_key: str | None = None, + api_key: str | SecretString | None = None, model: str | None = None, vertexai: bool | None = None, project: str | None = None, diff --git a/python/packages/gemini/tests/test_gemini_client.py b/python/packages/gemini/tests/test_gemini_client.py index 1b5d87b749c..4afeca620cb 100644 --- a/python/packages/gemini/tests/test_gemini_client.py +++ b/python/packages/gemini/tests/test_gemini_client.py @@ -12,6 +12,7 @@ import pytest from agent_framework import Agent, Content, FunctionTool, Message +from agent_framework._settings import SecretString from google.genai import types from pydantic import BaseModel from typing_extensions import NotRequired, TypedDict @@ -221,7 +222,10 @@ def test_client_created_from_api_key(monkeypatch: pytest.MonkeyPatch) -> None: assert client.model == "gemini-2.5-flash" -def test_client_created_from_google_api_key_env(monkeypatch: pytest.MonkeyPatch) -> None: +@pytest.mark.parametrize("api_key", [None, "explicit-key", SecretString("explicit-key")], ids=["env", "str", "secret"]) +def test_client_created_from_google_api_key_env( + monkeypatch: pytest.MonkeyPatch, api_key: str | SecretString | None +) -> None: """Initialises successfully when the SDK-standard Google API key environment variable is set.""" monkeypatch.delenv("GEMINI_API_KEY", raising=False) monkeypatch.delenv("GEMINI_MODEL", raising=False) @@ -237,9 +241,10 @@ def test_client_created_from_google_api_key_env(monkeypatch: pytest.MonkeyPatch) with patch("agent_framework_gemini._chat_client.genai.Client") as client_factory: client_factory.return_value = mock_client - client = GeminiChatClient() + client = GeminiChatClient(api_key=api_key) - assert client_factory.call_args.kwargs["api_key"] == "test-key-123" + assert type(client_factory.call_args.kwargs["api_key"]) is str + assert client_factory.call_args.kwargs["api_key"] == ("test-key-123" if api_key is None else "explicit-key") assert "vertexai" not in client_factory.call_args.kwargs assert client.model == "gemini-2.5-flash-lite" assert client.service_url() == "https://generativelanguage.googleapis.com" @@ -305,7 +310,10 @@ def test_missing_api_key_raises_when_no_client_injected(monkeypatch: pytest.Monk GeminiChatClient(model="gemini-2.5-flash") -def test_vertex_ai_express_mode_uses_api_key(monkeypatch: pytest.MonkeyPatch) -> None: +@pytest.mark.parametrize("api_key", [None, "explicit-key", SecretString("explicit-key")], ids=["env", "str", "secret"]) +def test_vertex_ai_express_mode_uses_api_key( + monkeypatch: pytest.MonkeyPatch, api_key: str | SecretString | None +) -> None: """Passes the API key in Vertex AI express mode when no project/location pair is configured.""" monkeypatch.delenv("GEMINI_API_KEY", raising=False) monkeypatch.delenv("GEMINI_MODEL", raising=False) @@ -319,10 +327,11 @@ def test_vertex_ai_express_mode_uses_api_key(monkeypatch: pytest.MonkeyPatch) -> mock_client._api_client._http_options.base_url = "https://aiplatform.googleapis.com/" with patch("agent_framework_gemini._chat_client.genai.Client", return_value=mock_client) as client_factory: - client = GeminiChatClient(model="gemini-2.5-flash-lite") + client = GeminiChatClient(model="gemini-2.5-flash-lite", api_key=api_key) assert client_factory.call_args.kwargs["vertexai"] is True - assert client_factory.call_args.kwargs["api_key"] == "test-key-123" + assert type(client_factory.call_args.kwargs["api_key"]) is str + assert client_factory.call_args.kwargs["api_key"] == ("test-key-123" if api_key is None else "explicit-key") assert "project" not in client_factory.call_args.kwargs assert "location" not in client_factory.call_args.kwargs assert client.service_url() == "https://aiplatform.googleapis.com" diff --git a/python/packages/mistral/tests/mistral/test_mistral_chat_client.py b/python/packages/mistral/tests/mistral/test_mistral_chat_client.py index 8d2888bf041..c1c86042a6f 100644 --- a/python/packages/mistral/tests/mistral/test_mistral_chat_client.py +++ b/python/packages/mistral/tests/mistral/test_mistral_chat_client.py @@ -9,6 +9,7 @@ import httpx import pytest from agent_framework import Agent, ChatResponse, Content, Message, tool +from agent_framework._settings import SecretString from agent_framework.exceptions import ( ChatClientException, ChatClientInvalidAuthException, @@ -134,11 +135,16 @@ def test_mistral_chat_construction_env(monkeypatch: pytest.MonkeyPatch) -> None: assert client.model == "mistral-large-latest" -def test_mistral_chat_construction_with_params() -> None: - client = MistralChatClient(model="mistral-large-latest", api_key="test-key") +@pytest.mark.parametrize("api_key", ["test-key", SecretString("test-key")], ids=["str", "secret"]) +def test_mistral_chat_construction_with_params(api_key: str | SecretString) -> None: + client = MistralChatClient(model="mistral-large-latest", api_key=api_key) assert client.model == "mistral-large-latest" assert isinstance(client.client, Mistral) assert client.client.sdk_configuration.timeout_ms == 60_000 + security = client.client.sdk_configuration.security + assert security is not None and not callable(security) + assert type(security.api_key) is str + assert security.api_key == "test-key" def test_mistral_chat_construction_with_server_url() -> None: diff --git a/python/packages/mistral/tests/mistral/test_mistral_embedding_client.py b/python/packages/mistral/tests/mistral/test_mistral_embedding_client.py index c08a033b0c9..eb6851e803b 100644 --- a/python/packages/mistral/tests/mistral/test_mistral_embedding_client.py +++ b/python/packages/mistral/tests/mistral/test_mistral_embedding_client.py @@ -9,6 +9,7 @@ import httpx import pytest from agent_framework import Embedding, GeneratedEmbeddings +from agent_framework._settings import SecretString from agent_framework.exceptions import ( IntegrationException, IntegrationInvalidAuthException, @@ -73,12 +74,17 @@ def test_mistral_embedding_construction(monkeypatch: pytest.MonkeyPatch) -> None assert client.model == "mistral-embed" -def test_mistral_embedding_construction_with_params() -> None: +@pytest.mark.parametrize("api_key", ["test-key", SecretString("test-key")], ids=["str", "secret"]) +def test_mistral_embedding_construction_with_params(api_key: str | SecretString) -> None: """Test construction with explicit parameters.""" - client = MistralEmbeddingClient(model="mistral-embed", api_key="test-key") + client = MistralEmbeddingClient(model="mistral-embed", api_key=api_key) assert client.model == "mistral-embed" assert isinstance(client.client, Mistral) assert client.client.sdk_configuration.timeout_ms == 60_000 + security = client.client.sdk_configuration.security + assert security is not None and not callable(security) + assert type(security.api_key) is str + assert security.api_key == "test-key" def test_mistral_embedding_construction_with_server_url() -> None: diff --git a/python/packages/openai/tests/openai/test_openai_shared.py b/python/packages/openai/tests/openai/test_openai_shared.py index f1036949b39..7f62cc491a0 100644 --- a/python/packages/openai/tests/openai/test_openai_shared.py +++ b/python/packages/openai/tests/openai/test_openai_shared.py @@ -10,16 +10,19 @@ import agent_framework._telemetry as telemetry import pytest from agent_framework import AGENT_FRAMEWORK_USER_AGENT +from agent_framework._settings import SecretString from agent_framework._telemetry import FeatureIndex as CoreFeatureIndex from agent_framework._telemetry import mark_feature_used from azure.core.credentials import TokenCredential from azure.core.credentials_async import AsyncTokenCredential +from openai import AsyncAzureOpenAI from agent_framework_openai._feature_usage import create_feature_usage_http_client from agent_framework_openai._shared import ( AZURE_OPENAI_TOKEN_SCOPE, _ensure_async_token_provider, _resolve_azure_credential_to_token_provider, + load_openai_service_settings, ) @@ -44,6 +47,35 @@ def get_token(self, *scopes: str, **kwargs: object): raise NotImplementedError +@pytest.mark.usefixtures("openai_unit_test_env") +@pytest.mark.parametrize("api_key", ["test-secret-key", SecretString("test-secret-key")], ids=["str", "secret"]) +@pytest.mark.parametrize("route", ["openai", "azure"]) +async def test_service_settings_unwrap_api_key_at_sdk_boundary(api_key: str | SecretString, route: str) -> None: + settings, sdk_client, use_azure = load_openai_service_settings( + model="test-model", + api_key=api_key, + credential=None, + org_id=None, + base_url=None, + endpoint="https://test.openai.azure.com" if route != "openai" else None, + api_version=None, + default_azure_api_version="2024-12-01-preview", + env_file_path=None, + env_file_encoding=None, + ) + try: + assert use_azure is (route != "openai") + assert isinstance(sdk_client, AsyncAzureOpenAI) is (route == "azure") + assert type(sdk_client.api_key) is str + assert sdk_client.api_key == "test-secret-key" + assert isinstance(settings["api_key"], SecretString) + assert settings["api_key"].get_secret_value() == "test-secret-key" + assert "test-secret-key" not in str(settings["api_key"]) + assert "test-secret-key" not in repr(settings) + finally: + await sdk_client.close() + + def test_resolve_azure_async_credential_wraps_provider() -> None: credential = _AsyncTokenCredentialStub() token_provider = MagicMock()