refactor(backend): add description field support for secrets (v1 conversations) (#12080)

This commit is contained in:
Hiep Le
2025-12-29 22:43:07 +07:00
committed by GitHub
parent 8d69b4066f
commit d3afbfa447
3 changed files with 162 additions and 31 deletions

View File

@@ -579,6 +579,7 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
continue continue
secret_name = f'{provider_type.name}_TOKEN' secret_name = f'{provider_type.name}_TOKEN'
description = f'{provider_type.name} authentication token'
if self.web_url: if self.web_url:
# Create an access token for web-based authentication # Create an access token for web-based authentication
@@ -598,12 +599,15 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
secrets[secret_name] = LookupSecret( secrets[secret_name] = LookupSecret(
url=self.web_url + '/api/v1/webhooks/secrets', url=self.web_url + '/api/v1/webhooks/secrets',
headers=headers, headers=headers,
description=description,
) )
else: else:
# Use static token for environments without web URL access # Use static token for environments without web URL access
static_token = await self.user_context.get_latest_token(provider_type) static_token = await self.user_context.get_latest_token(provider_type)
if static_token: if static_token:
secrets[secret_name] = StaticSecret(value=static_token) secrets[secret_name] = StaticSecret(
value=static_token, description=description
)
return secrets return secrets

View File

@@ -81,7 +81,12 @@ class AuthUserContext(UserContext):
secrets = await self.user_auth.get_secrets() secrets = await self.user_auth.get_secrets()
if secrets: if secrets:
for name, custom_secret in secrets.custom_secrets.items(): for name, custom_secret in secrets.custom_secrets.items():
results[name] = StaticSecret(value=custom_secret.secret) results[name] = StaticSecret(
value=custom_secret.secret,
description=custom_secret.description
if custom_secret.description
else None,
)
return results return results

View File

@@ -8,6 +8,7 @@ from unittest.mock import AsyncMock, Mock, patch
from uuid import UUID, uuid4 from uuid import UUID, uuid4
import pytest import pytest
from pydantic import SecretStr
from openhands.agent_server.models import ( from openhands.agent_server.models import (
SendMessageRequest, SendMessageRequest,
@@ -29,7 +30,7 @@ from openhands.app_server.sandbox.sandbox_models import (
) )
from openhands.app_server.sandbox.sandbox_spec_models import SandboxSpecInfo from openhands.app_server.sandbox.sandbox_spec_models import SandboxSpecInfo
from openhands.app_server.user.user_context import UserContext from openhands.app_server.user.user_context import UserContext
from openhands.integrations.provider import ProviderType from openhands.integrations.provider import ProviderToken, ProviderType
from openhands.sdk import Agent, Event from openhands.sdk import Agent, Event
from openhands.sdk.llm import LLM from openhands.sdk.llm import LLM
from openhands.sdk.secret import LookupSecret, StaticSecret from openhands.sdk.secret import LookupSecret, StaticSecret
@@ -114,10 +115,6 @@ class TestLiveStatusAppConversationService:
async def test_setup_secrets_for_git_providers_with_web_url(self): async def test_setup_secrets_for_git_providers_with_web_url(self):
"""Test _setup_secrets_for_git_providers with web URL (creates access token).""" """Test _setup_secrets_for_git_providers with web URL (creates access token)."""
# Arrange # Arrange
from pydantic import SecretStr
from openhands.integrations.provider import ProviderToken
base_secrets = {} base_secrets = {}
self.mock_user_context.get_secrets.return_value = base_secrets self.mock_user_context.get_secrets.return_value = base_secrets
self.mock_jwt_service.create_jws_token.return_value = 'test_access_token' self.mock_jwt_service.create_jws_token.return_value = 'test_access_token'
@@ -144,6 +141,9 @@ class TestLiveStatusAppConversationService:
== 'https://test.example.com/api/v1/webhooks/secrets' == 'https://test.example.com/api/v1/webhooks/secrets'
) )
assert result['GITHUB_TOKEN'].headers['X-Access-Token'] == 'test_access_token' assert result['GITHUB_TOKEN'].headers['X-Access-Token'] == 'test_access_token'
# Verify descriptions are included
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
assert result['GITLAB_TOKEN'].description == 'GITLAB authentication token'
# Should be called twice, once for each provider # Should be called twice, once for each provider
assert self.mock_jwt_service.create_jws_token.call_count == 2 assert self.mock_jwt_service.create_jws_token.call_count == 2
@@ -152,10 +152,6 @@ class TestLiveStatusAppConversationService:
async def test_setup_secrets_for_git_providers_with_saas_mode(self): async def test_setup_secrets_for_git_providers_with_saas_mode(self):
"""Test _setup_secrets_for_git_providers with SaaS mode (includes keycloak cookie).""" """Test _setup_secrets_for_git_providers with SaaS mode (includes keycloak cookie)."""
# Arrange # Arrange
from pydantic import SecretStr
from openhands.integrations.provider import ProviderToken
self.service.app_mode = 'saas' self.service.app_mode = 'saas'
self.service.keycloak_auth_cookie = 'test_cookie' self.service.keycloak_auth_cookie = 'test_cookie'
base_secrets = {} base_secrets = {}
@@ -179,15 +175,13 @@ class TestLiveStatusAppConversationService:
assert isinstance(lookup_secret, LookupSecret) assert isinstance(lookup_secret, LookupSecret)
assert 'Cookie' in lookup_secret.headers assert 'Cookie' in lookup_secret.headers
assert lookup_secret.headers['Cookie'] == 'keycloak_auth=test_cookie' assert lookup_secret.headers['Cookie'] == 'keycloak_auth=test_cookie'
# Verify description is included
assert lookup_secret.description == 'GITLAB authentication token'
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_setup_secrets_for_git_providers_without_web_url(self): async def test_setup_secrets_for_git_providers_without_web_url(self):
"""Test _setup_secrets_for_git_providers without web URL (uses static token).""" """Test _setup_secrets_for_git_providers without web URL (uses static token)."""
# Arrange # Arrange
from pydantic import SecretStr
from openhands.integrations.provider import ProviderToken
self.service.web_url = None self.service.web_url = None
base_secrets = {} base_secrets = {}
self.mock_user_context.get_secrets.return_value = base_secrets self.mock_user_context.get_secrets.return_value = base_secrets
@@ -208,6 +202,8 @@ class TestLiveStatusAppConversationService:
assert 'GITHUB_TOKEN' in result assert 'GITHUB_TOKEN' in result
assert isinstance(result['GITHUB_TOKEN'], StaticSecret) assert isinstance(result['GITHUB_TOKEN'], StaticSecret)
assert result['GITHUB_TOKEN'].value.get_secret_value() == 'static_token_value' assert result['GITHUB_TOKEN'].value.get_secret_value() == 'static_token_value'
# Verify description is included
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
self.mock_user_context.get_latest_token.assert_called_once_with( self.mock_user_context.get_latest_token.assert_called_once_with(
ProviderType.GITHUB ProviderType.GITHUB
) )
@@ -216,10 +212,6 @@ class TestLiveStatusAppConversationService:
async def test_setup_secrets_for_git_providers_no_static_token(self): async def test_setup_secrets_for_git_providers_no_static_token(self):
"""Test _setup_secrets_for_git_providers when no static token is available.""" """Test _setup_secrets_for_git_providers when no static token is available."""
# Arrange # Arrange
from pydantic import SecretStr
from openhands.integrations.provider import ProviderToken
self.service.web_url = None self.service.web_url = None
base_secrets = {} base_secrets = {}
self.mock_user_context.get_secrets.return_value = base_secrets self.mock_user_context.get_secrets.return_value = base_secrets
@@ -240,6 +232,148 @@ class TestLiveStatusAppConversationService:
assert 'GITHUB_TOKEN' not in result assert 'GITHUB_TOKEN' not in result
assert result == base_secrets assert result == base_secrets
@pytest.mark.asyncio
async def test_setup_secrets_for_git_providers_descriptions_included(self):
"""Test _setup_secrets_for_git_providers includes descriptions for all provider types."""
# Arrange
base_secrets = {}
self.mock_user_context.get_secrets.return_value = base_secrets
self.mock_jwt_service.create_jws_token.return_value = 'test_access_token'
# Mock provider tokens for multiple providers
provider_tokens = {
ProviderType.GITHUB: ProviderToken(token=SecretStr('github_token')),
ProviderType.GITLAB: ProviderToken(token=SecretStr('gitlab_token')),
ProviderType.BITBUCKET: ProviderToken(token=SecretStr('bitbucket_token')),
}
self.mock_user_context.get_provider_tokens = AsyncMock(
return_value=provider_tokens
)
# Act
result = await self.service._setup_secrets_for_git_providers(self.mock_user)
# Assert - verify all secrets have correct descriptions
assert 'GITHUB_TOKEN' in result
assert isinstance(result['GITHUB_TOKEN'], LookupSecret)
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
assert 'GITLAB_TOKEN' in result
assert isinstance(result['GITLAB_TOKEN'], LookupSecret)
assert result['GITLAB_TOKEN'].description == 'GITLAB authentication token'
assert 'BITBUCKET_TOKEN' in result
assert isinstance(result['BITBUCKET_TOKEN'], LookupSecret)
assert result['BITBUCKET_TOKEN'].description == 'BITBUCKET authentication token'
@pytest.mark.asyncio
async def test_setup_secrets_for_git_providers_static_secret_description(self):
"""Test _setup_secrets_for_git_providers includes description for StaticSecret."""
# Arrange
self.service.web_url = None
base_secrets = {}
self.mock_user_context.get_secrets.return_value = base_secrets
self.mock_user_context.get_latest_token.return_value = 'static_token_value'
# Mock provider tokens for multiple providers
provider_tokens = {
ProviderType.GITHUB: ProviderToken(token=SecretStr('github_token')),
ProviderType.GITLAB: ProviderToken(token=SecretStr('gitlab_token')),
}
self.mock_user_context.get_provider_tokens = AsyncMock(
return_value=provider_tokens
)
# Act
result = await self.service._setup_secrets_for_git_providers(self.mock_user)
# Assert - verify StaticSecret objects have descriptions
assert 'GITHUB_TOKEN' in result
assert isinstance(result['GITHUB_TOKEN'], StaticSecret)
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
assert 'GITLAB_TOKEN' in result
assert isinstance(result['GITLAB_TOKEN'], StaticSecret)
assert result['GITLAB_TOKEN'].description == 'GITLAB authentication token'
@pytest.mark.asyncio
async def test_setup_secrets_for_git_providers_preserves_custom_secret_descriptions(
self,
):
"""Test _setup_secrets_for_git_providers preserves descriptions from custom secrets."""
# Arrange
# Mock custom secrets with descriptions
custom_secret_with_desc = StaticSecret(
value=SecretStr('custom_secret_value'),
description='Custom API key for external service',
)
custom_secret_no_desc = StaticSecret(
value=SecretStr('another_secret_value'),
description=None,
)
base_secrets = {
'CUSTOM_API_KEY': custom_secret_with_desc,
'ANOTHER_SECRET': custom_secret_no_desc,
}
self.mock_user_context.get_secrets.return_value = base_secrets
self.mock_jwt_service.create_jws_token.return_value = 'test_access_token'
# Mock provider tokens
provider_tokens = {
ProviderType.GITHUB: ProviderToken(token=SecretStr('github_token')),
}
self.mock_user_context.get_provider_tokens = AsyncMock(
return_value=provider_tokens
)
# Act
result = await self.service._setup_secrets_for_git_providers(self.mock_user)
# Assert - verify custom secrets are preserved with their descriptions
assert 'CUSTOM_API_KEY' in result
assert isinstance(result['CUSTOM_API_KEY'], StaticSecret)
assert (
result['CUSTOM_API_KEY'].description
== 'Custom API key for external service'
)
assert (
result['CUSTOM_API_KEY'].value.get_secret_value() == 'custom_secret_value'
)
assert 'ANOTHER_SECRET' in result
assert isinstance(result['ANOTHER_SECRET'], StaticSecret)
assert result['ANOTHER_SECRET'].description is None
assert (
result['ANOTHER_SECRET'].value.get_secret_value() == 'another_secret_value'
)
# Verify git provider token is also included
assert 'GITHUB_TOKEN' in result
assert result['GITHUB_TOKEN'].description == 'GITHUB authentication token'
@pytest.mark.asyncio
async def test_setup_secrets_for_git_providers_custom_secret_empty_description(
self,
):
"""Test _setup_secrets_for_git_providers handles custom secrets with empty descriptions."""
# Arrange
custom_secret_empty_desc = StaticSecret(
value=SecretStr('secret_value'),
description='', # Empty string description
)
base_secrets = {'MY_SECRET': custom_secret_empty_desc}
self.mock_user_context.get_secrets.return_value = base_secrets
self.mock_user_context.get_provider_tokens = AsyncMock(return_value=None)
# Act
result = await self.service._setup_secrets_for_git_providers(self.mock_user)
# Assert - empty description should be preserved as-is
assert 'MY_SECRET' in result
assert isinstance(result['MY_SECRET'], StaticSecret)
# Empty string description is preserved
assert result['MY_SECRET'].description == ''
@pytest.mark.asyncio @pytest.mark.asyncio
async def test_configure_llm_and_mcp_with_custom_model(self): async def test_configure_llm_and_mcp_with_custom_model(self):
"""Test _configure_llm_and_mcp with custom LLM model.""" """Test _configure_llm_and_mcp with custom LLM model."""
@@ -370,8 +504,6 @@ class TestLiveStatusAppConversationService:
async def test_configure_llm_and_mcp_tavily_with_user_search_api_key(self): async def test_configure_llm_and_mcp_tavily_with_user_search_api_key(self):
"""Test _configure_llm_and_mcp adds tavily when user has search_api_key.""" """Test _configure_llm_and_mcp adds tavily when user has search_api_key."""
# Arrange # Arrange
from pydantic import SecretStr
self.mock_user.search_api_key = SecretStr('user_search_key') self.mock_user.search_api_key = SecretStr('user_search_key')
self.mock_user_context.get_mcp_api_key.return_value = 'mcp_api_key' self.mock_user_context.get_mcp_api_key.return_value = 'mcp_api_key'
@@ -416,8 +548,6 @@ class TestLiveStatusAppConversationService:
async def test_configure_llm_and_mcp_tavily_user_key_takes_precedence(self): async def test_configure_llm_and_mcp_tavily_user_key_takes_precedence(self):
"""Test _configure_llm_and_mcp user search_api_key takes precedence over env key.""" """Test _configure_llm_and_mcp user search_api_key takes precedence over env key."""
# Arrange # Arrange
from pydantic import SecretStr
self.mock_user.search_api_key = SecretStr('user_search_key') self.mock_user.search_api_key = SecretStr('user_search_key')
self.service.tavily_api_key = 'env_tavily_key' self.service.tavily_api_key = 'env_tavily_key'
self.mock_user_context.get_mcp_api_key.return_value = None self.mock_user_context.get_mcp_api_key.return_value = None
@@ -486,8 +616,6 @@ class TestLiveStatusAppConversationService:
Even in SAAS mode, if the user has their own search_api_key, tavily should be added. Even in SAAS mode, if the user has their own search_api_key, tavily should be added.
""" """
# Arrange - simulate SAAS mode with user having their own search key # Arrange - simulate SAAS mode with user having their own search key
from pydantic import SecretStr
self.service.app_mode = AppMode.SAAS.value self.service.app_mode = AppMode.SAAS.value
self.service.tavily_api_key = None # In SAAS mode, this should be None self.service.tavily_api_key = None # In SAAS mode, this should be None
self.mock_user.search_api_key = SecretStr('user_search_key') self.mock_user.search_api_key = SecretStr('user_search_key')
@@ -512,8 +640,6 @@ class TestLiveStatusAppConversationService:
async def test_configure_llm_and_mcp_tavily_with_empty_user_search_key(self): async def test_configure_llm_and_mcp_tavily_with_empty_user_search_key(self):
"""Test _configure_llm_and_mcp handles empty user search_api_key correctly.""" """Test _configure_llm_and_mcp handles empty user search_api_key correctly."""
# Arrange # Arrange
from pydantic import SecretStr
self.mock_user.search_api_key = SecretStr('') # Empty string self.mock_user.search_api_key = SecretStr('') # Empty string
self.service.tavily_api_key = 'env_tavily_key' self.service.tavily_api_key = 'env_tavily_key'
self.mock_user_context.get_mcp_api_key.return_value = None self.mock_user_context.get_mcp_api_key.return_value = None
@@ -537,8 +663,6 @@ class TestLiveStatusAppConversationService:
async def test_configure_llm_and_mcp_tavily_with_whitespace_user_search_key(self): async def test_configure_llm_and_mcp_tavily_with_whitespace_user_search_key(self):
"""Test _configure_llm_and_mcp handles whitespace-only user search_api_key correctly.""" """Test _configure_llm_and_mcp handles whitespace-only user search_api_key correctly."""
# Arrange # Arrange
from pydantic import SecretStr
self.mock_user.search_api_key = SecretStr(' ') # Whitespace only self.mock_user.search_api_key = SecretStr(' ') # Whitespace only
self.service.tavily_api_key = 'env_tavily_key' self.service.tavily_api_key = 'env_tavily_key'
self.mock_user_context.get_mcp_api_key.return_value = None self.mock_user_context.get_mcp_api_key.return_value = None
@@ -1314,8 +1438,6 @@ class TestLiveStatusAppConversationService:
async def test_configure_llm_and_mcp_merges_system_and_custom_servers(self): async def test_configure_llm_and_mcp_merges_system_and_custom_servers(self):
"""Test _configure_llm_and_mcp merges both system and custom MCP servers.""" """Test _configure_llm_and_mcp merges both system and custom MCP servers."""
# Arrange # Arrange
from pydantic import SecretStr
from openhands.core.config.mcp_config import ( from openhands.core.config.mcp_config import (
MCPConfig, MCPConfig,
MCPSSEServerConfig, MCPSSEServerConfig,