From f5228406fe275f824ad9772cd41a385d5cd1fbaf Mon Sep 17 00:00:00 2001 From: eavanvalkenburg Date: Mon, 7 Sep 2026 16:13:06 +0200 Subject: [PATCH 1/3] [BREAKING] Python: Refine SecretString handling Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../_bedrock_client.py | 12 +- .../agent_framework_anthropic/_chat_client.py | 4 +- .../_foundry_client.py | 4 +- .../anthropic/tests/test_anthropic_client.py | 17 +- .../tests/test_anthropic_provider_clients.py | 23 ++- .../_context_provider.py | 26 +-- .../tests/test_aisearch_context_provider.py | 31 +++- .../_checkpoint_storage.py | 6 +- .../_history_provider.py | 6 +- .../tests/test_cosmos_checkpoint_storage.py | 22 ++- .../tests/test_cosmos_history_provider.py | 20 ++- .../agent_framework_bedrock/_chat_client.py | 6 +- .../_embedding_client.py | 12 +- .../bedrock/tests/test_bedrock_client.py | 37 +++- python/packages/core/AGENTS.md | 8 + .../core/agent_framework/_settings.py | 82 +++++++-- .../packages/core/tests/core/test_settings.py | 158 +++++++++++++++++- .../agent_framework_gemini/_chat_client.py | 4 +- .../gemini/tests/test_gemini_client.py | 21 ++- .../tests/mistral/test_mistral_chat_client.py | 10 +- .../mistral/test_mistral_embedding_client.py | 10 +- .../openai/tests/openai/test_openai_shared.py | 32 ++++ 22 files changed, 460 insertions(+), 91 deletions(-) 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 7b0cf4eb792..cefc50b1223 100644 --- a/python/packages/anthropic/agent_framework_anthropic/_chat_client.py +++ b/python/packages/anthropic/agent_framework_anthropic/_chat_client.py @@ -297,7 +297,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, @@ -1671,7 +1671,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 bd9eba7f16b..3ff2c944937 100644 --- a/python/packages/anthropic/tests/test_anthropic_client.py +++ b/python/packages/anthropic/tests/test_anthropic_client.py @@ -23,7 +23,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.exceptions import ( ChatClientException, @@ -101,17 +101,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" @@ -160,16 +163,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 8b64ab24ed3..fea571a5c84 100644 --- a/python/packages/core/AGENTS.md +++ b/python/packages/core/AGENTS.md @@ -38,6 +38,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 17b2feafdb6..a6c6796b418 100644 --- a/python/packages/gemini/agent_framework_gemini/_chat_client.py +++ b/python/packages/gemini/agent_framework_gemini/_chat_client.py @@ -345,7 +345,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, @@ -1390,7 +1390,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 74b1687e1d3..f9f3f774d3d 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 agent_framework.exceptions import ( ChatClientException, ChatClientInvalidAuthException, @@ -227,7 +228,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) @@ -243,9 +247,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" @@ -311,7 +316,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) @@ -325,10 +333,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() From 18cc266227bc6be2b7b66ede26552e9dc4c41915 Mon Sep 17 00:00:00 2001 From: eavanvalkenburg Date: Mon, 7 Sep 2026 20:36:34 +0200 Subject: [PATCH 2/3] Python: Address SecretString review feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../core/agent_framework/_settings.py | 8 ++++++- .../packages/core/tests/core/test_settings.py | 10 +++++++++ .../_embedding_client.py | 11 +++++----- .../foundry/test_foundry_embedding_client.py | 21 ++++++++++++++++++- .../agent_framework_openai/_chat_client.py | 6 +++--- .../_chat_completion_client.py | 6 +++--- .../_embedding_client.py | 6 +++--- .../openai/test_openai_embedding_client.py | 13 +++++++++++- 8 files changed, 64 insertions(+), 17 deletions(-) diff --git a/python/packages/core/agent_framework/_settings.py b/python/packages/core/agent_framework/_settings.py index bcd6828d892..ea7fcb10ea7 100644 --- a/python/packages/core/agent_framework/_settings.py +++ b/python/packages/core/agent_framework/_settings.py @@ -78,12 +78,18 @@ class SecretString: __slots__ = ("_value",) + _value: str + 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 + object.__setattr__(self, "_value", value) + + def __setattr__(self, name: str, value: object) -> None: + """Reject mutation after construction.""" + raise AttributeError("SecretString is immutable.") def __str__(self) -> str: """Return a masked string to prevent secret exposure.""" diff --git a/python/packages/core/tests/core/test_settings.py b/python/packages/core/tests/core/test_settings.py index fb810c9188c..00794f0d3d3 100644 --- a/python/packages/core/tests/core/test_settings.py +++ b/python/packages/core/tests/core/test_settings.py @@ -296,6 +296,16 @@ def test_secretstring_value_semantics(self, value: str) -> None: assert hash(secret) == hash(value) assert {secret, SecretString(value), value} == {value} + def test_secretstring_is_immutable(self) -> None: + secret = SecretString("my-secret") + secrets = {secret} + + with pytest.raises(AttributeError, match="^SecretString is immutable\\.$"): + secret._value = "another-secret" + + assert secret.get_secret_value() == "my-secret" + assert secret in secrets + def test_secretstring_can_wrap_existing_secret(self) -> None: secret = SecretString(SecretString("my-secret")) diff --git a/python/packages/foundry/agent_framework_foundry/_embedding_client.py b/python/packages/foundry/agent_framework_foundry/_embedding_client.py index 779738e9150..c5254ad890a 100644 --- a/python/packages/foundry/agent_framework_foundry/_embedding_client.py +++ b/python/packages/foundry/agent_framework_foundry/_embedding_client.py @@ -14,6 +14,7 @@ Embedding, EmbeddingGenerationOptions, GeneratedEmbeddings, + SecretString, UsageDetails, load_settings, ) @@ -83,7 +84,7 @@ class FoundryEmbeddingSettings(TypedDict, total=False): """Foundry inference embedding settings.""" models_endpoint: str | None - models_api_key: str | None + models_api_key: SecretString | None embedding_model: str | None image_embedding_model: str | None @@ -123,7 +124,7 @@ def __init__( model: str | None = None, image_model: str | None = None, endpoint: str | None = None, - api_key: str | None = None, + api_key: str | SecretString | None = None, text_client: EmbeddingsClient | None = None, image_client: ImageEmbeddingsClient | None = None, credential: AzureKeyCredential | None = None, @@ -148,8 +149,8 @@ def __init__( self.image_model: str = settings.get("image_embedding_model") or self.model # type: ignore[assignment] resolved_endpoint = settings["models_endpoint"] # type: ignore[reportTypedDictNotRequiredAccess] - if credential is None and settings.get("models_api_key"): - credential = AzureKeyCredential(settings["models_api_key"]) # type: ignore[arg-type] + if credential is None and (models_api_key := settings.get("models_api_key")): + credential = AzureKeyCredential(models_api_key.get_secret_value()) if credential is None and text_client is None and image_client is None: raise ValueError("Either 'api_key', 'credential', or pre-configured client(s) must be provided.") @@ -381,7 +382,7 @@ def __init__( model: str | None = None, image_model: str | None = None, endpoint: str | None = None, - api_key: str | None = None, + api_key: str | SecretString | None = None, text_client: EmbeddingsClient | None = None, image_client: ImageEmbeddingsClient | None = None, credential: AzureKeyCredential | None = None, diff --git a/python/packages/foundry/tests/foundry/test_foundry_embedding_client.py b/python/packages/foundry/tests/foundry/test_foundry_embedding_client.py index 827f2ea4343..2eec8ea0672 100644 --- a/python/packages/foundry/tests/foundry/test_foundry_embedding_client.py +++ b/python/packages/foundry/tests/foundry/test_foundry_embedding_client.py @@ -8,8 +8,9 @@ from unittest.mock import ANY, AsyncMock, MagicMock, patch import pytest -from agent_framework import Content +from agent_framework import Content, SecretString from agent_framework._telemetry import get_user_agent +from azure.core.credentials import AzureKeyCredential from agent_framework_foundry import ( FoundryEmbeddingClient, @@ -279,6 +280,24 @@ async def test_text_embeddings(self, client: FoundryEmbeddingClient[Any], mock_t assert len(result) == 1 assert result[0].vector == [0.1, 0.2, 0.3] + def test_accepts_secret_string_api_key(self) -> None: + with ( + patch("agent_framework_foundry._embedding_client.EmbeddingsClient") as text_client_type, + patch("agent_framework_foundry._embedding_client.ImageEmbeddingsClient") as image_client_type, + ): + FoundryEmbeddingClient( + model="test-model", + endpoint="https://test.inference.ai.azure.com", + api_key=SecretString("test-key"), + ) + + text_credential = text_client_type.call_args.kwargs["credential"] + image_credential = image_client_type.call_args.kwargs["credential"] + assert isinstance(text_credential, AzureKeyCredential) + assert text_credential is image_credential + assert type(text_credential.key) is str + assert text_credential.key == "test-key" + async def test_otel_provider_name_default(self) -> None: """Default OTEL provider name is azure.ai.inference.""" assert FoundryEmbeddingClient.OTEL_PROVIDER_NAME == "azure.ai.inference" diff --git a/python/packages/openai/agent_framework_openai/_chat_client.py b/python/packages/openai/agent_framework_openai/_chat_client.py index a9120e72ca0..0cc2bc3e34c 100644 --- a/python/packages/openai/agent_framework_openai/_chat_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_client.py @@ -3609,7 +3609,7 @@ def __init__( self, model: str | None = None, *, - api_key: str | Callable[[], str | Awaitable[str]] | None = None, + api_key: str | SecretString | Callable[[], str | Awaitable[str]] | None = None, org_id: str | None = None, base_url: str | None = None, default_headers: Mapping[str, str] | None = None, @@ -3656,7 +3656,7 @@ def __init__( azure_endpoint: str | None = None, credential: AzureCredentialTypes | AzureTokenProvider | None = None, api_version: str | None = None, - api_key: str | Callable[[], str | Awaitable[str]] | None = None, + api_key: str | SecretString | Callable[[], str | Awaitable[str]] | None = None, base_url: str | None = None, default_headers: Mapping[str, str] | None = None, async_client: AsyncAzureOpenAI | AsyncOpenAI | None = None, @@ -3705,7 +3705,7 @@ def __init__( self, model: str | None = None, *, - api_key: str | Callable[[], str | Awaitable[str]] | None = None, + api_key: str | SecretString | Callable[[], str | Awaitable[str]] | None = None, credential: AzureCredentialTypes | AzureTokenProvider | None = None, org_id: str | None = None, base_url: str | None = None, diff --git a/python/packages/openai/agent_framework_openai/_chat_completion_client.py b/python/packages/openai/agent_framework_openai/_chat_completion_client.py index 0682753ed39..700825d67f8 100644 --- a/python/packages/openai/agent_framework_openai/_chat_completion_client.py +++ b/python/packages/openai/agent_framework_openai/_chat_completion_client.py @@ -1362,7 +1362,7 @@ def __init__( self, model: str | None = None, *, - api_key: str | Callable[[], str | Awaitable[str]] | None = None, + api_key: str | SecretString | Callable[[], str | Awaitable[str]] | None = None, org_id: str | None = None, base_url: str | None = None, default_headers: Mapping[str, str] | None = None, @@ -1409,7 +1409,7 @@ def __init__( azure_endpoint: str | None = None, credential: AzureCredentialTypes | AzureTokenProvider | None = None, api_version: str | None = None, - api_key: str | Callable[[], str | Awaitable[str]] | None = None, + api_key: str | SecretString | Callable[[], str | Awaitable[str]] | None = None, base_url: str | None = None, default_headers: Mapping[str, str] | None = None, async_client: AsyncAzureOpenAI | AsyncOpenAI | None = None, @@ -1458,7 +1458,7 @@ def __init__( self, model: str | None = None, *, - api_key: str | Callable[[], str | Awaitable[str]] | None = None, + api_key: str | SecretString | Callable[[], str | Awaitable[str]] | None = None, credential: AzureCredentialTypes | AzureTokenProvider | None = None, org_id: str | None = None, default_headers: Mapping[str, str] | None = None, diff --git a/python/packages/openai/agent_framework_openai/_embedding_client.py b/python/packages/openai/agent_framework_openai/_embedding_client.py index d2b84d1f420..c3ab22bdd13 100644 --- a/python/packages/openai/agent_framework_openai/_embedding_client.py +++ b/python/packages/openai/agent_framework_openai/_embedding_client.py @@ -331,7 +331,7 @@ def __init__( self, *, model: str | None = None, - api_key: str | Callable[[], str | Awaitable[str]] | None = None, + api_key: str | SecretString | Callable[[], str | Awaitable[str]] | None = None, org_id: str | None = None, default_headers: Mapping[str, str] | None = None, async_client: AsyncOpenAI | None = None, @@ -368,7 +368,7 @@ def __init__( azure_endpoint: str | None = None, credential: AzureCredentialTypes | AzureTokenProvider | None = None, api_version: str | None = None, - api_key: str | Callable[[], str | Awaitable[str]] | None = None, + api_key: str | SecretString | Callable[[], str | Awaitable[str]] | None = None, base_url: str | None = None, default_headers: Mapping[str, str] | None = None, async_client: AsyncAzureOpenAI | AsyncOpenAI | None = None, @@ -407,7 +407,7 @@ def __init__( self, *, model: str | None = None, - api_key: str | Callable[[], str | Awaitable[str]] | None = None, + api_key: str | SecretString | Callable[[], str | Awaitable[str]] | None = None, credential: AzureCredentialTypes | AzureTokenProvider | None = None, org_id: str | None = None, default_headers: Mapping[str, str] | None = None, diff --git a/python/packages/openai/tests/openai/test_openai_embedding_client.py b/python/packages/openai/tests/openai/test_openai_embedding_client.py index 629c47a79ac..ba57b8ee754 100644 --- a/python/packages/openai/tests/openai/test_openai_embedding_client.py +++ b/python/packages/openai/tests/openai/test_openai_embedding_client.py @@ -9,7 +9,7 @@ import agent_framework._telemetry as telemetry import pytest -from agent_framework import SupportsGetEmbeddings +from agent_framework import SecretString, SupportsGetEmbeddings from agent_framework._telemetry import get_feature_token from agent_framework.exceptions import SettingNotFoundError from openai.types import CreateEmbeddingResponse @@ -52,6 +52,17 @@ def test_openai_construction_with_explicit_params() -> None: assert isinstance(client, SupportsGetEmbeddings) +def test_public_openai_embedding_client_accepts_secret_string() -> None: + client = OpenAIEmbeddingClient( + model="text-embedding-3-small", + api_key=SecretString("test-key"), + ) + + assert client.client is not None + assert type(client.client.api_key) is str + assert client.client.api_key == "test-key" + + def test_raw_openai_embedding_client_init_uses_explicit_parameters() -> None: signature = inspect.signature(RawOpenAIEmbeddingClient.__init__) From 2caf482859078ee4a348bbc9eae61f62e5bbc227 Mon Sep 17 00:00:00 2001 From: eavanvalkenburg Date: Tue, 8 Sep 2026 08:15:10 +0200 Subject: [PATCH 3/3] Python: Address additional SecretString feedback Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../agent_framework_anthropic/_vertex_client.py | 11 +++++++---- .../anthropic/tests/test_anthropic_client.py | 7 ++++--- .../tests/test_anthropic_provider_clients.py | 9 +++++++-- .../agent_framework_copilotstudio/_agent.py | 6 ++++-- .../copilotstudio/tests/test_copilot_agent.py | 15 ++++++++++++++- python/packages/core/agent_framework/_settings.py | 8 ++++++++ python/packages/core/tests/core/test_sessions.py | 13 +++++++++++++ python/packages/core/tests/core/test_settings.py | 13 +++++++++++-- python/packages/core/tests/workflow/test_state.py | 9 +++++++++ .../agent_framework_mem0/_context_provider.py | 7 ++++--- .../mem0/tests/test_mem0_context_provider.py | 8 +++++--- 11 files changed, 86 insertions(+), 20 deletions(-) diff --git a/python/packages/anthropic/agent_framework_anthropic/_vertex_client.py b/python/packages/anthropic/agent_framework_anthropic/_vertex_client.py index 77edeb5f529..71e9b8ef69c 100644 --- a/python/packages/anthropic/agent_framework_anthropic/_vertex_client.py +++ b/python/packages/anthropic/agent_framework_anthropic/_vertex_client.py @@ -11,7 +11,7 @@ FunctionInvocationConfiguration, FunctionInvocationLayer, ) -from agent_framework._settings import load_settings +from agent_framework._settings import SecretString, load_settings from agent_framework._telemetry import get_user_agent from agent_framework.observability import ChatTelemetryLayer from anthropic import NOT_GIVEN @@ -43,7 +43,7 @@ def __init__( model: str | None = None, region: str | None = None, project_id: str | None = None, - access_token: str | None = None, + access_token: str | SecretString | None = None, credentials: GoogleCredentials | None = None, base_url: str | None = None, anthropic_client: AsyncAnthropicVertex | None = None, @@ -84,10 +84,13 @@ def __init__( if anthropic_client is None: resolved_region = region_setting if region_setting is not None else NOT_GIVEN resolved_project_id = project_id_setting if project_id_setting is not None else NOT_GIVEN + resolved_access_token = ( + access_token.get_secret_value() if isinstance(access_token, SecretString) else access_token + ) anthropic_client = AsyncAnthropicVertex( region=resolved_region, project_id=resolved_project_id, - access_token=access_token, + access_token=resolved_access_token, credentials=credentials, base_url=settings.get("anthropic_vertex_base_url"), default_headers={"User-Agent": get_user_agent()}, @@ -116,7 +119,7 @@ def __init__( model: str | None = None, region: str | None = None, project_id: str | None = None, - access_token: str | None = None, + access_token: str | SecretString | None = None, credentials: GoogleCredentials | None = None, base_url: str | None = None, anthropic_client: AsyncAnthropicVertex | None = None, diff --git a/python/packages/anthropic/tests/test_anthropic_client.py b/python/packages/anthropic/tests/test_anthropic_client.py index 3ff2c944937..91d8d8b7187 100644 --- a/python/packages/anthropic/tests/test_anthropic_client.py +++ b/python/packages/anthropic/tests/test_anthropic_client.py @@ -174,9 +174,10 @@ def test_anthropic_client_init_auto_create_client( 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"] + anthropic_client = client.anthropic_client + assert isinstance(anthropic_client, anthropic_sdk.AsyncAnthropic) + assert type(anthropic_client.api_key) is str + assert 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 580acaff552..d637aedef96 100644 --- a/python/packages/anthropic/tests/test_anthropic_provider_clients.py +++ b/python/packages/anthropic/tests/test_anthropic_provider_clients.py @@ -211,7 +211,10 @@ def test_raw_anthropic_bedrock_client_creates_sdk_client_from_arguments( assert type(factory.call_args.kwargs[key]) is str -def test_raw_anthropic_vertex_client_creates_sdk_client_from_arguments() -> None: +@pytest.mark.parametrize("access_token", ["access-token", SecretString("access-token")], ids=["str", "secret"]) +def test_raw_anthropic_vertex_client_creates_sdk_client_from_arguments( + access_token: str | SecretString, +) -> None: mock_transport = _create_mock_transport("https://us-central1-aiplatform.googleapis.com/v1") with patch("agent_framework_anthropic._vertex_client.AsyncAnthropicVertex", return_value=mock_transport) as factory: @@ -219,6 +222,7 @@ def test_raw_anthropic_vertex_client_creates_sdk_client_from_arguments() -> None model="claude-vertex-test", region="us-central1", project_id="test-project", + access_token=access_token, ) assert client.model == "claude-vertex-test" @@ -226,8 +230,9 @@ def test_raw_anthropic_vertex_client_creates_sdk_client_from_arguments() -> None factory.assert_called_once_with( region="us-central1", project_id="test-project", - access_token=None, + access_token="access-token", credentials=None, base_url=None, default_headers={"User-Agent": get_user_agent()}, ) + assert type(factory.call_args.kwargs["access_token"]) is str diff --git a/python/packages/copilotstudio/agent_framework_copilotstudio/_agent.py b/python/packages/copilotstudio/agent_framework_copilotstudio/_agent.py index 8121784e17c..f276395f722 100644 --- a/python/packages/copilotstudio/agent_framework_copilotstudio/_agent.py +++ b/python/packages/copilotstudio/agent_framework_copilotstudio/_agent.py @@ -15,6 +15,7 @@ ContextProvider, Message, ResponseStream, + SecretString, normalize_messages, ) from agent_framework._settings import load_settings @@ -75,7 +76,7 @@ def __init__( agent_identifier: str | None = None, client_id: str | None = None, tenant_id: str | None = None, - token: str | None = None, + token: str | SecretString | None = None, cloud: PowerPlatformCloud | None = None, agent_type: AgentType | None = None, custom_power_platform_cloud: str | None = None, @@ -206,7 +207,8 @@ def __init__( scopes=scopes, ) - client = CopilotClient(settings=settings, token=token) + resolved_token = token.get_secret_value() if isinstance(token, SecretString) else token + client = CopilotClient(settings=settings, token=resolved_token) self.client = client self.cloud = cloud diff --git a/python/packages/copilotstudio/tests/test_copilot_agent.py b/python/packages/copilotstudio/tests/test_copilot_agent.py index a661aeca26d..47dfe3a45bf 100644 --- a/python/packages/copilotstudio/tests/test_copilot_agent.py +++ b/python/packages/copilotstudio/tests/test_copilot_agent.py @@ -4,7 +4,7 @@ from unittest.mock import MagicMock, patch import pytest -from agent_framework import AgentResponse, AgentResponseUpdate, AgentSession, Content, Message +from agent_framework import AgentResponse, AgentResponseUpdate, AgentSession, Content, Message, SecretString from agent_framework.exceptions import AgentException from microsoft_agents.copilotstudio.client import ConnectionSettings, CopilotClient @@ -100,6 +100,19 @@ def test_init_with_client(self, mock_copilot_client: MagicMock) -> None: assert agent.client == mock_copilot_client assert agent.id is not None + @pytest.mark.parametrize("token", ["fake-token", SecretString("fake-token")], ids=["str", "secret"]) + @patch("agent_framework_copilotstudio._agent.CopilotClient") + def test_init_unwraps_token_for_client(self, client_type: MagicMock, token: str | SecretString) -> None: + CopilotStudioAgent( + environment_id="env-id", + agent_identifier="agent-id", + token=token, + ) + + client_type.assert_called_once() + assert type(client_type.call_args.kwargs["token"]) is str + assert client_type.call_args.kwargs["token"] == "fake-token" + def test_init_applies_default_read_bufsize(self) -> None: """The internally built client raises aiohttp's per-line limit to avoid LineTooLong (issue #7257).""" agent = CopilotStudioAgent( diff --git a/python/packages/core/agent_framework/_settings.py b/python/packages/core/agent_framework/_settings.py index ea7fcb10ea7..1629eb5ff96 100644 --- a/python/packages/core/agent_framework/_settings.py +++ b/python/packages/core/agent_framework/_settings.py @@ -91,6 +91,14 @@ def __setattr__(self, name: str, value: object) -> None: """Reject mutation after construction.""" raise AttributeError("SecretString is immutable.") + def __copy__(self) -> SecretString: + """Return this immutable instance for shallow copies.""" + return self + + def __deepcopy__(self, memo: dict[int, Any]) -> SecretString: + """Return this immutable instance for deep copies.""" + return self + def __str__(self) -> str: """Return a masked string to prevent secret exposure.""" return "**********" diff --git a/python/packages/core/tests/core/test_sessions.py b/python/packages/core/tests/core/test_sessions.py index 138e8a0fc18..9b789496a09 100644 --- a/python/packages/core/tests/core/test_sessions.py +++ b/python/packages/core/tests/core/test_sessions.py @@ -27,6 +27,7 @@ HistoryProvider, InMemoryHistoryProvider, Message, + SecretString, SessionContext, SessionStore, agent_middleware, @@ -884,6 +885,18 @@ async def test_set_then_get_returns_independent_copy(self) -> None: assert reread is not None assert reread.state["nested"]["values"] == ["original"] + async def test_set_and_get_preserves_immutable_secret_string(self) -> None: + store = SessionStore() + secret = SecretString("my-secret") + session = AgentSession(session_id="session-1") + session.state["secret"] = secret + + await store.set("session-1", session) + + stored = await store.get("session-1") + assert stored is not None + assert stored.state["secret"] is secret + async def test_set_stores_independent_snapshot(self) -> None: store = SessionStore() session = AgentSession(session_id="session-1") diff --git a/python/packages/core/tests/core/test_settings.py b/python/packages/core/tests/core/test_settings.py index 00794f0d3d3..9070c31a6ed 100644 --- a/python/packages/core/tests/core/test_settings.py +++ b/python/packages/core/tests/core/test_settings.py @@ -7,6 +7,7 @@ import os import tempfile from collections.abc import Callable +from copy import copy, deepcopy from pathlib import Path from typing import Any, Literal, TypedDict @@ -172,9 +173,11 @@ def test_secretstring_from_wrapped_override(self) -> None: secret = SecretString("my-secret") settings = load_settings(SecretSettings, env_prefix="SECRET_", api_key=secret) + resolved_api_key = settings["api_key"] - assert settings["api_key"] is secret - assert settings["api_key"].get_secret_value() == "my-secret" + assert resolved_api_key is secret + assert isinstance(resolved_api_key, SecretString) + assert resolved_api_key.get_secret_value() == "my-secret" def test_secretstring_from_dotenv(self, tmp_path: Path) -> None: env_file = tmp_path / ".env" @@ -306,6 +309,12 @@ def test_secretstring_is_immutable(self) -> None: assert secret.get_secret_value() == "my-secret" assert secret in secrets + def test_secretstring_copy_returns_same_immutable_instance(self) -> None: + secret = SecretString("my-secret") + + assert copy(secret) is secret + assert deepcopy(secret) is secret + def test_secretstring_can_wrap_existing_secret(self) -> None: secret = SecretString(SecretString("my-secret")) diff --git a/python/packages/core/tests/workflow/test_state.py b/python/packages/core/tests/workflow/test_state.py index db9dd17ee50..e233eb976f2 100644 --- a/python/packages/core/tests/workflow/test_state.py +++ b/python/packages/core/tests/workflow/test_state.py @@ -4,6 +4,7 @@ import pytest +from agent_framework import SecretString from agent_framework._workflows._state import State @@ -15,6 +16,14 @@ def test_set_and_get(self) -> None: state.set("key", "value") assert state.get("key") == "value" + def test_set_and_get_secret_string(self) -> None: + state = State() + secret = SecretString("my-secret") + + state.set("key", secret) + + assert state.get("key") is secret + def test_set_does_not_alias_caller_value(self) -> None: state = State() value = {"history": ["step-1"]} diff --git a/python/packages/mem0/agent_framework_mem0/_context_provider.py b/python/packages/mem0/agent_framework_mem0/_context_provider.py index 20bba5ad9e1..d89a110733d 100644 --- a/python/packages/mem0/agent_framework_mem0/_context_provider.py +++ b/python/packages/mem0/agent_framework_mem0/_context_provider.py @@ -15,7 +15,7 @@ from contextlib import AbstractAsyncContextManager from typing import TYPE_CHECKING, Any, ClassVar, TypeAlias, TypedDict -from agent_framework import Message +from agent_framework import Message, SecretString from agent_framework._sessions import AgentSession, ContextProvider, SessionContext from agent_framework._telemetry import mark_feature_used from mem0 import AsyncMemory, AsyncMemoryClient @@ -68,7 +68,7 @@ def __init__( self, source_id: str = DEFAULT_SOURCE_ID, mem0_client: AsyncMemory | AsyncMemoryClient | None = None, - api_key: str | None = None, + api_key: str | SecretString | None = None, application_id: str | None = None, agent_id: str | None = None, user_id: str | None = None, @@ -105,7 +105,8 @@ def __init__( super().__init__(source_id) should_close_client = False if mem0_client is None: - mem0_client = AsyncMemoryClient(api_key=api_key) + resolved_api_key = api_key.get_secret_value() if isinstance(api_key, SecretString) else api_key + mem0_client = AsyncMemoryClient(api_key=resolved_api_key) should_close_client = True self.api_key = api_key diff --git a/python/packages/mem0/tests/test_mem0_context_provider.py b/python/packages/mem0/tests/test_mem0_context_provider.py index bde518c56d1..9318cbf3103 100644 --- a/python/packages/mem0/tests/test_mem0_context_provider.py +++ b/python/packages/mem0/tests/test_mem0_context_provider.py @@ -7,7 +7,7 @@ from unittest.mock import AsyncMock, MagicMock, patch import pytest -from agent_framework import AgentResponse, Message +from agent_framework import AgentResponse, Message, SecretString from agent_framework._sessions import AgentSession, SessionContext from agent_framework_mem0._context_provider import Mem0ContextProvider @@ -82,14 +82,16 @@ def test_init_default_context_prompt(self, mock_mem0_client: AsyncMock) -> None: provider = Mem0ContextProvider(source_id="mem0", mem0_client=mock_mem0_client, user_id="u1") assert provider.context_prompt == Mem0ContextProvider.DEFAULT_CONTEXT_PROMPT - def test_init_auto_creates_client_when_none(self) -> None: + @pytest.mark.parametrize("api_key", ["test-key", SecretString("test-key")], ids=["str", "secret"]) + def test_init_auto_creates_client_when_none(self, api_key: str | SecretString) -> None: """When no client is provided, a default AsyncMemoryClient is created and flagged for closing.""" with ( patch("mem0.client.main.AsyncMemoryClient.__init__", return_value=None) as mock_init, patch("mem0.client.main.AsyncMemoryClient._validate_api_key", return_value=None), ): - provider = Mem0ContextProvider(source_id="mem0", api_key="test-key", user_id="u1") + provider = Mem0ContextProvider(source_id="mem0", api_key=api_key, user_id="u1") mock_init.assert_called_once_with(api_key="test-key") + assert type(mock_init.call_args.kwargs["api_key"]) is str assert provider._should_close_client is True def test_provided_client_not_flagged_for_close(self, mock_mem0_client: AsyncMock) -> None: