Add server-side automatic conversion of string secrets to StaticSecret

openhands-agent-server/openhands/agent_server/models.py

@@ -1,10 +1,10 @@
 from abc import ABC
 from datetime import datetime
 from enum import Enum
-from typing import Literal
+from typing import Any, Literal
 from uuid import uuid4
 
-from pydantic import BaseModel, Field
+from pydantic import BaseModel, Field, field_validator
 
 from openhands.agent_server.utils import OpenHandsUUID, utc_now
 from openhands.sdk import LLM, AgentBase, Event, ImageContent, Message, TextContent
@@ -159,6 +159,40 @@ class UpdateSecretsRequest(BaseModel):
         description="Dictionary mapping secret keys to values"
     )
 
+    @field_validator("secrets", mode="before")
+    @classmethod
+    def convert_string_secrets(cls, v: Any) -> dict[str, Any]:
+        """Convert plain string secrets to StaticSecret objects.
+
+        This allows clients to send either:
+        - Plain strings: {"API_KEY": "secret-value"}
+        - SecretSource objects: {"API_KEY": {"kind": "StaticSecret", "value": "..."}}
+        """
+        if not isinstance(v, dict):
+            return v
+
+        result = {}
+        for key, value in v.items():
+            if isinstance(value, str):
+                # Convert plain string to StaticSecret dict format
+                result[key] = {"kind": "StaticSecret", "value": value}
+            elif isinstance(value, dict) and "kind" not in value:
+                # Handle case where client sends {"value": "secret"} without kind
+                if "value" in value:
+                    result[key] = {
+                        "kind": "StaticSecret",
+                        "value": value["value"],
+                        "description": value.get("description"),
+                    }
+                else:
+                    # Invalid dict format - add a default kind to prevent KeyError
+                    # This will still fail validation later with a better error message
+                    result[key] = {"kind": "StaticSecret", **value}
+            else:
+                # Already a SecretSource object or proper dict format
+                result[key] = value
+        return result
+
 
 class SetConfirmationPolicyRequest(BaseModel):
     """Payload to set confirmation policy for a conversation."""

tests/agent_server/test_conversation_router.py

@@ -1265,3 +1265,117 @@ def test_security_analyzer_endpoint_with_malformed_analyzer_data(
     assert response.status_code == 422
     response_data = response.json()
     assert "detail" in response_data
+
+
+def test_update_conversation_secrets_string_conversion(
+    client, mock_conversation_service, mock_event_service, sample_conversation_id
+):
+    """Test update_conversation_secrets endpoint with plain string secrets.
+
+    This test verifies that plain string secrets are automatically converted
+    to StaticSecret objects on the server side.
+    """
+
+    # Mock the service responses
+    mock_conversation_service.get_event_service.return_value = mock_event_service
+    mock_event_service.update_secrets.return_value = None
+
+    client.app.dependency_overrides[get_conversation_service] = (
+        lambda: mock_conversation_service
+    )
+
+    try:
+        # Send plain string secrets (this should work with the new validator)
+        request_data = {
+            "secrets": {
+                "API_KEY": "plain-secret-value",
+                "TOKEN": "plain-token-value",
+            }
+        }
+
+        response = client.post(
+            f"/api/conversations/{sample_conversation_id}/secrets", json=request_data
+        )
+
+        assert response.status_code == 200
+        data = response.json()
+        assert data["success"] is True
+
+        # Verify services were called
+        mock_conversation_service.get_event_service.assert_called_once_with(
+            sample_conversation_id
+        )
+        mock_event_service.update_secrets.assert_called_once()
+
+        # Verify that the secrets were converted to proper SecretSource objects
+        call_args = mock_event_service.update_secrets.call_args[0][0]
+        assert "API_KEY" in call_args
+        assert "TOKEN" in call_args
+
+        # The values should be SecretSource objects, not plain strings
+        from openhands.sdk.conversation.secret_source import StaticSecret
+
+        assert isinstance(call_args["API_KEY"], StaticSecret)
+        assert isinstance(call_args["TOKEN"], StaticSecret)
+
+        # Verify the actual secret values
+        assert call_args["API_KEY"].get_value() == "plain-secret-value"
+        assert call_args["TOKEN"].get_value() == "plain-token-value"
+
+    finally:
+        client.app.dependency_overrides.clear()
+
+
+def test_update_conversation_secrets_mixed_formats(
+    client, mock_conversation_service, mock_event_service, sample_conversation_id
+):
+    """Test update_conversation_secrets endpoint with mixed secret formats.
+
+    This test verifies that the server can handle a mix of plain strings
+    and properly formatted SecretSource objects.
+    """
+
+    # Mock the service responses
+    mock_conversation_service.get_event_service.return_value = mock_event_service
+    mock_event_service.update_secrets.return_value = None
+
+    client.app.dependency_overrides[get_conversation_service] = (
+        lambda: mock_conversation_service
+    )
+
+    try:
+        # Mix of plain strings and proper SecretSource objects
+        request_data = {
+            "secrets": {
+                "PLAIN_SECRET": "plain-value",
+                "STATIC_SECRET": {"kind": "StaticSecret", "value": "static-value"},
+                "LOOKUP_SECRET": {
+                    "kind": "LookupSecret",
+                    "url": "https://example.com/secret",
+                },
+            }
+        }
+
+        response = client.post(
+            f"/api/conversations/{sample_conversation_id}/secrets", json=request_data
+        )
+
+        assert response.status_code == 200
+        data = response.json()
+        assert data["success"] is True
+
+        # Verify services were called
+        mock_conversation_service.get_event_service.assert_called_once_with(
+            sample_conversation_id
+        )
+        mock_event_service.update_secrets.assert_called_once()
+
+        # Verify that all secrets were processed correctly
+        call_args = mock_event_service.update_secrets.call_args[0][0]
+        assert len(call_args) == 3
+        assert "PLAIN_SECRET" in call_args
+        assert "STATIC_SECRET" in call_args
+        assert "LOOKUP_SECRET" in call_args
+
+    finally:
+        client.app.dependency_overrides.clear()

tests/agent_server/test_models.py

@@ -0,0 +1,115 @@
+"""Tests for agent_server models."""
+
+from typing import Any
+
+import pytest
+from pydantic import SecretStr, ValidationError
+
+from openhands.agent_server.models import UpdateSecretsRequest
+from openhands.sdk.conversation.secret_source import LookupSecret, StaticSecret
+
+
+def test_update_secrets_request_string_conversion():
+    """Test that plain string secrets are converted to StaticSecret objects."""
+
+    # Test with plain string secrets
+    request = UpdateSecretsRequest(
+        secrets={  # type: ignore[arg-type]
+            "API_KEY": "plain-secret-value",
+            "TOKEN": "another-secret",
+        }
+    )
+
+    # Verify conversion happened
+    assert isinstance(request.secrets["API_KEY"], StaticSecret)
+    assert isinstance(request.secrets["TOKEN"], StaticSecret)
+
+    # Verify values are correct
+    assert request.secrets["API_KEY"].get_value() == "plain-secret-value"
+    assert request.secrets["TOKEN"].get_value() == "another-secret"
+
+
+def test_update_secrets_request_proper_secret_source():
+    """Test that properly formatted SecretSource objects are preserved."""
+
+    # Test with properly formatted SecretSource objects
+    request = UpdateSecretsRequest(
+        secrets={
+            "STATIC_SECRET": StaticSecret(value=SecretStr("static-value")),
+            "LOOKUP_SECRET": LookupSecret(url="https://example.com/secret"),
+        }
+    )
+
+    # Verify objects are preserved as-is
+    assert isinstance(request.secrets["STATIC_SECRET"], StaticSecret)
+    assert isinstance(request.secrets["LOOKUP_SECRET"], LookupSecret)
+
+    # Verify values
+    assert request.secrets["STATIC_SECRET"].get_value() == "static-value"
+    assert request.secrets["LOOKUP_SECRET"].url == "https://example.com/secret"
+
+
+def test_update_secrets_request_mixed_formats():
+    """Test that mixed formats (strings and SecretSource objects) work together."""
+
+    secrets_dict: dict[str, Any] = {
+        "PLAIN_SECRET": "plain-value",
+        "STATIC_SECRET": StaticSecret(value=SecretStr("static-value")),
+        "LOOKUP_SECRET": LookupSecret(url="https://example.com/secret"),
+    }
+    request = UpdateSecretsRequest(secrets=secrets_dict)  # type: ignore[arg-type]
+
+    # Verify all types are correct
+    assert isinstance(request.secrets["PLAIN_SECRET"], StaticSecret)
+    assert isinstance(request.secrets["STATIC_SECRET"], StaticSecret)
+    assert isinstance(request.secrets["LOOKUP_SECRET"], LookupSecret)
+
+    # Verify values
+    assert request.secrets["PLAIN_SECRET"].get_value() == "plain-value"
+    assert request.secrets["STATIC_SECRET"].get_value() == "static-value"
+    assert request.secrets["LOOKUP_SECRET"].url == "https://example.com/secret"
+
+
+def test_update_secrets_request_dict_without_kind():
+    """Test handling of dict values without 'kind' field."""
+
+    request = UpdateSecretsRequest(
+        secrets={  # type: ignore[arg-type]
+            "SECRET_WITH_VALUE": {
+                "value": "secret-value",
+                "description": "A test secret",
+            },
+        }
+    )
+
+    # Secret with value should be converted to StaticSecret
+    assert isinstance(request.secrets["SECRET_WITH_VALUE"], StaticSecret)
+    assert request.secrets["SECRET_WITH_VALUE"].get_value() == "secret-value"
+    assert request.secrets["SECRET_WITH_VALUE"].description == "A test secret"
+
+
+def test_update_secrets_request_invalid_dict():
+    """Test handling of invalid dict values without 'kind' or 'value' field."""
+
+    # This should raise a validation error since the dict is invalid
+    with pytest.raises(ValidationError):
+        UpdateSecretsRequest(
+            secrets={  # type: ignore[arg-type]
+                "SECRET_WITHOUT_VALUE": {"description": "No value"},
+            }
+        )
+
+
+def test_update_secrets_request_empty_secrets():
+    """Test that empty secrets dict is handled correctly."""
+
+    request = UpdateSecretsRequest(secrets={})
+    assert request.secrets == {}
+
+
+def test_update_secrets_request_invalid_input():
+    """Test that invalid input types are handled appropriately."""
+
+    # Non-dict input should be preserved (will fail validation later)
+    with pytest.raises(ValidationError):
+        UpdateSecretsRequest(secrets="not-a-dict")  # type: ignore[arg-type]