From 75c823c4862eda39180d214a3ab359f63add3151 Mon Sep 17 00:00:00 2001 From: Xingyao Wang Date: Tue, 17 Mar 2026 12:54:57 +0000 Subject: [PATCH] feat: expose_secrets param on /users/me + sandbox-scoped secrets API (#13383) Co-authored-by: openhands --- AGENTS.md | 27 + enterprise/integrations/resolver_context.py | 4 +- .../live_status_app_conversation_service.py | 9 +- .../app_server/sandbox/sandbox_models.py | 17 + .../app_server/sandbox/sandbox_router.py | 121 ++- openhands/app_server/sandbox/session_auth.py | 66 ++ .../app_server/user/auth_user_context.py | 27 +- .../app_server/user/specifiy_user_context.py | 4 +- openhands/app_server/user/user_context.py | 12 +- openhands/app_server/user/user_router.py | 51 +- .../app_server/test_sandbox_secrets_router.py | 787 ++++++++++++++++++ 11 files changed, 1110 insertions(+), 15 deletions(-) create mode 100644 openhands/app_server/sandbox/session_auth.py create mode 100644 tests/unit/app_server/test_sandbox_secrets_router.py diff --git a/AGENTS.md b/AGENTS.md index 878a26e884..5bcf99cf8b 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -342,3 +342,30 @@ To add a new LLM model to OpenHands, you need to update multiple files across bo - Models appear in CLI provider selection based on the verified arrays - The `organize_models_and_providers` function groups models by provider - Default model selection prioritizes verified models for each provider + +### Sandbox Settings API (SDK Credential Inheritance) + +The sandbox settings API allows SDK-created conversations to inherit the user's SaaS credentials +(LLM config, secrets) securely via `LookupSecret`. Raw secret values only flow SaaS→sandbox, +never through the SDK client. + +#### User Credentials with Exposed Secrets (in `openhands/app_server/user/user_router.py`): +- `GET /api/v1/users/me?expose_secrets=true` → Full user settings with unmasked secrets (e.g., `llm_api_key`) +- `GET /api/v1/users/me` → Full user settings (secrets masked, Bearer only) + +Auth requirements for `expose_secrets=true`: +- Bearer token (proves user identity via `OPENHANDS_API_KEY`) +- `X-Session-API-Key` header (proves caller has an active sandbox owned by the authenticated user) + +Called by `workspace.get_llm()` in the SDK to retrieve LLM config with the API key. + +#### Sandbox-Scoped Secrets Endpoints (in `openhands/app_server/sandbox/sandbox_router.py`): +- `GET /sandboxes/{id}/settings/secrets` → list secret names (no values) +- `GET /sandboxes/{id}/settings/secrets/{name}` → raw secret value (called FROM sandbox) + +#### Auth: `X-Session-API-Key` header, validated via `SandboxService.get_sandbox_by_session_api_key()` + +#### Related SDK code (in `software-agent-sdk` repo): +- `openhands/sdk/llm/llm.py`: `LLM.api_key` accepts `SecretSource` (including `LookupSecret`) +- `openhands/workspace/cloud/workspace.py`: `get_llm()` and `get_secrets()` return LookupSecret-backed objects +- Tests: `tests/sdk/llm/test_llm_secret_source_api_key.py`, `tests/workspace/test_cloud_workspace_sdk_settings.py` diff --git a/enterprise/integrations/resolver_context.py b/enterprise/integrations/resolver_context.py index 441c132f2c..30e558a111 100644 --- a/enterprise/integrations/resolver_context.py +++ b/enterprise/integrations/resolver_context.py @@ -60,7 +60,9 @@ class ResolverUserContext(UserContext): return provider_token.token.get_secret_value() return None - async def get_provider_tokens(self) -> PROVIDER_TOKEN_TYPE | None: + async def get_provider_tokens( + self, as_env_vars: bool = False + ) -> PROVIDER_TOKEN_TYPE | dict[str, str] | None: return await self.saas_user_auth.get_provider_tokens() async def get_secrets(self) -> dict[str, SecretSource]: diff --git a/openhands/app_server/app_conversation/live_status_app_conversation_service.py b/openhands/app_server/app_conversation/live_status_app_conversation_service.py index 902cde7771..703899ec83 100644 --- a/openhands/app_server/app_conversation/live_status_app_conversation_service.py +++ b/openhands/app_server/app_conversation/live_status_app_conversation_service.py @@ -7,7 +7,7 @@ import zipfile from collections import defaultdict from dataclasses import dataclass from datetime import datetime, timedelta -from typing import Any, AsyncGenerator, Sequence +from typing import Any, AsyncGenerator, Sequence, cast from uuid import UUID, uuid4 import httpx @@ -84,7 +84,7 @@ from openhands.app_server.utils.llm_metadata import ( get_llm_metadata, should_set_litellm_extra_body, ) -from openhands.integrations.provider import ProviderType +from openhands.integrations.provider import PROVIDER_TOKEN_TYPE, ProviderType from openhands.integrations.service_types import SuggestedTask from openhands.sdk import Agent, AgentContext, LocalWorkspace from openhands.sdk.hooks import HookConfig @@ -837,7 +837,10 @@ class LiveStatusAppConversationService(AppConversationServiceBase): secrets = await self.user_context.get_secrets() # Get all provider tokens from user authentication - provider_tokens = await self.user_context.get_provider_tokens() + provider_tokens = cast( + PROVIDER_TOKEN_TYPE | None, + await self.user_context.get_provider_tokens(), + ) if not provider_tokens: return secrets diff --git a/openhands/app_server/sandbox/sandbox_models.py b/openhands/app_server/sandbox/sandbox_models.py index 948e5c17c2..c64d188b34 100644 --- a/openhands/app_server/sandbox/sandbox_models.py +++ b/openhands/app_server/sandbox/sandbox_models.py @@ -59,3 +59,20 @@ class SandboxInfo(BaseModel): class SandboxPage(BaseModel): items: list[SandboxInfo] next_page_id: str | None = None + + +class SecretNameItem(BaseModel): + """A secret's name and optional description (value NOT included).""" + + name: str = Field(description='The secret name/key') + description: str | None = Field( + default=None, description='Optional description of the secret' + ) + + +class SecretNamesResponse(BaseModel): + """Response listing available secret names (no raw values).""" + + secrets: list[SecretNameItem] = Field( + default_factory=list, description='Available secrets' + ) diff --git a/openhands/app_server/sandbox/sandbox_router.py b/openhands/app_server/sandbox/sandbox_router.py index 79a3ef6b82..7b2575c3e7 100644 --- a/openhands/app_server/sandbox/sandbox_router.py +++ b/openhands/app_server/sandbox/sandbox_router.py @@ -1,16 +1,30 @@ """Runtime Containers router for OpenHands App Server.""" -from typing import Annotated +import logging +from typing import Annotated, cast -from fastapi import APIRouter, HTTPException, Query, status +from fastapi import APIRouter, Depends, HTTPException, Query, Request, Response, status +from fastapi.security import APIKeyHeader from openhands.agent_server.models import Success from openhands.app_server.config import depends_sandbox_service -from openhands.app_server.sandbox.sandbox_models import SandboxInfo, SandboxPage +from openhands.app_server.sandbox.sandbox_models import ( + SandboxInfo, + SandboxPage, + SecretNameItem, + SecretNamesResponse, +) from openhands.app_server.sandbox.sandbox_service import ( SandboxService, ) +from openhands.app_server.sandbox.session_auth import validate_session_key +from openhands.app_server.user.auth_user_context import AuthUserContext from openhands.server.dependencies import get_dependencies +from openhands.server.user_auth.user_auth import ( + get_for_user as get_user_auth_for_user, +) + +_logger = logging.getLogger(__name__) # We use the get_dependencies method here to signal to the OpenAPI docs that this endpoint # is protected. The actual protection is provided by SetAuthCookieMiddleware @@ -94,3 +108,104 @@ async def delete_sandbox( if not exists: raise HTTPException(status.HTTP_404_NOT_FOUND) return Success() + + +# --------------------------------------------------------------------------- +# Sandbox-scoped secrets (authenticated via X-Session-API-Key) +# --------------------------------------------------------------------------- + + +async def _valid_sandbox_from_session_key( + request: Request, + sandbox_id: str, + session_api_key: str = Depends( + APIKeyHeader(name='X-Session-API-Key', auto_error=False) + ), +) -> SandboxInfo: + """Authenticate via ``X-Session-API-Key`` and verify sandbox ownership.""" + sandbox_info = await validate_session_key(session_api_key) + + if sandbox_info.id != sandbox_id: + raise HTTPException( + status.HTTP_403_FORBIDDEN, + detail='Session API key does not match sandbox', + ) + + return sandbox_info + + +async def _get_user_context(sandbox_info: SandboxInfo) -> AuthUserContext: + """Build an ``AuthUserContext`` for the sandbox owner.""" + if not sandbox_info.created_by_user_id: + raise HTTPException( + status.HTTP_401_UNAUTHORIZED, + detail='Sandbox has no associated user', + ) + user_auth = await get_user_auth_for_user(sandbox_info.created_by_user_id) + return AuthUserContext(user_auth=user_auth) + + +@router.get('/{sandbox_id}/settings/secrets') +async def list_secret_names( + sandbox_info: SandboxInfo = Depends(_valid_sandbox_from_session_key), +) -> SecretNamesResponse: + """List available secret names (no raw values). + + Includes both custom secrets and provider tokens (e.g. github_token). + """ + user_context = await _get_user_context(sandbox_info) + + items: list[SecretNameItem] = [] + + # Custom secrets + secret_sources = await user_context.get_secrets() + for name, source in secret_sources.items(): + items.append(SecretNameItem(name=name, description=source.description)) + + # Provider tokens (github_token, gitlab_token, etc.) + provider_env_vars = cast( + dict[str, str] | None, + await user_context.get_provider_tokens(as_env_vars=True), + ) + if provider_env_vars: + for env_key in provider_env_vars: + items.append( + SecretNameItem(name=env_key, description=f'{env_key} provider token') + ) + + return SecretNamesResponse(secrets=items) + + +@router.get('/{sandbox_id}/settings/secrets/{secret_name}') +async def get_secret_value( + secret_name: str, + sandbox_info: SandboxInfo = Depends(_valid_sandbox_from_session_key), +) -> Response: + """Return a single secret value as plain text. + + Called by ``LookupSecret`` inside the sandbox. Checks custom secrets + first, then falls back to provider tokens — always resolving the + latest token at call time. + """ + user_context = await _get_user_context(sandbox_info) + + # Check custom secrets first + secret_sources = await user_context.get_secrets() + source = secret_sources.get(secret_name) + if source is not None: + value = source.get_value() + if value is None: + raise HTTPException(status.HTTP_404_NOT_FOUND, detail='Secret has no value') + return Response(content=value, media_type='text/plain') + + # Fall back to provider tokens (resolved fresh per request) + provider_env_vars = cast( + dict[str, str] | None, + await user_context.get_provider_tokens(as_env_vars=True), + ) + if provider_env_vars: + token_value = provider_env_vars.get(secret_name) + if token_value is not None: + return Response(content=token_value, media_type='text/plain') + + raise HTTPException(status.HTTP_404_NOT_FOUND, detail='Secret not found') diff --git a/openhands/app_server/sandbox/session_auth.py b/openhands/app_server/sandbox/session_auth.py new file mode 100644 index 0000000000..5ae7f418b6 --- /dev/null +++ b/openhands/app_server/sandbox/session_auth.py @@ -0,0 +1,66 @@ +"""Shared session-key authentication for sandbox-scoped endpoints. + +Both the sandbox router and the user router need to validate +``X-Session-API-Key`` headers. This module centralises that logic so +it lives in exactly one place. + +The ``InjectorState`` + ``ADMIN`` pattern used here is established in +``webhook_router.py`` — the sandbox service requires an admin context to +look up sandboxes across all users by session key, but the session key +itself acts as the proof of access. +""" + +import logging + +from fastapi import HTTPException, status + +from openhands.app_server.config import get_global_config, get_sandbox_service +from openhands.app_server.sandbox.sandbox_models import SandboxInfo +from openhands.app_server.services.injector import InjectorState +from openhands.app_server.user.specifiy_user_context import ADMIN, USER_CONTEXT_ATTR +from openhands.server.types import AppMode + +_logger = logging.getLogger(__name__) + + +async def validate_session_key(session_api_key: str | None) -> SandboxInfo: + """Validate an ``X-Session-API-Key`` and return the associated sandbox. + + Raises: + HTTPException(401): if the key is missing or does not map to a sandbox. + HTTPException(401): in SAAS mode if the sandbox has no owning user. + """ + if not session_api_key: + raise HTTPException( + status.HTTP_401_UNAUTHORIZED, + detail='X-Session-API-Key header is required', + ) + + # The sandbox service is scoped to users. To look up a sandbox by session + # key (which could belong to *any* user) we need an admin context. This + # is the same pattern used in webhook_router.valid_sandbox(). + state = InjectorState() + setattr(state, USER_CONTEXT_ATTR, ADMIN) + + async with get_sandbox_service(state) as sandbox_service: + sandbox_info = await sandbox_service.get_sandbox_by_session_api_key( + session_api_key + ) + + if sandbox_info is None: + raise HTTPException( + status.HTTP_401_UNAUTHORIZED, detail='Invalid session API key' + ) + + if not sandbox_info.created_by_user_id: + if get_global_config().app_mode == AppMode.SAAS: + _logger.error( + 'Sandbox had no user specified', + extra={'sandbox_id': sandbox_info.id}, + ) + raise HTTPException( + status.HTTP_401_UNAUTHORIZED, + detail='Sandbox had no user specified', + ) + + return sandbox_info diff --git a/openhands/app_server/user/auth_user_context.py b/openhands/app_server/user/auth_user_context.py index c6f7df07de..663ab0bbfd 100644 --- a/openhands/app_server/user/auth_user_context.py +++ b/openhands/app_server/user/auth_user_context.py @@ -48,8 +48,27 @@ class AuthUserContext(UserContext): self._user_info = user_info return user_info - async def get_provider_tokens(self) -> PROVIDER_TOKEN_TYPE | None: - return await self.user_auth.get_provider_tokens() + async def get_provider_tokens( + self, as_env_vars: bool = False + ) -> PROVIDER_TOKEN_TYPE | dict[str, str] | None: + """Return provider tokens. + + Args: + as_env_vars: When True, return a ``dict[str, str]`` mapping env + var names (e.g. ``github_token``) to plain-text token values, + resolving the latest value at call time. When False (default), + return the raw ``dict[ProviderType, ProviderToken]``. + """ + provider_tokens = await self.user_auth.get_provider_tokens() + if not as_env_vars: + return provider_tokens + results: dict[str, str] = {} + if provider_tokens: + for provider_type, provider_token in provider_tokens.items(): + if provider_token.token: + env_key = ProviderHandler.get_provider_env_key(provider_type) + results[env_key] = provider_token.token.get_secret_value() + return results async def get_provider_handler(self): provider_handler = self._provider_handler @@ -79,9 +98,9 @@ class AuthUserContext(UserContext): return token async def get_secrets(self) -> dict[str, SecretSource]: - results = {} + results: dict[str, SecretSource] = {} - # Include custom secrets... + # Include custom secrets secrets = await self.user_auth.get_secrets() if secrets: for name, custom_secret in secrets.custom_secrets.items(): diff --git a/openhands/app_server/user/specifiy_user_context.py b/openhands/app_server/user/specifiy_user_context.py index 0127d13129..087aac9e7f 100644 --- a/openhands/app_server/user/specifiy_user_context.py +++ b/openhands/app_server/user/specifiy_user_context.py @@ -26,7 +26,9 @@ class SpecifyUserContext(UserContext): ) -> str: raise NotImplementedError() - async def get_provider_tokens(self) -> PROVIDER_TOKEN_TYPE | None: + async def get_provider_tokens( + self, as_env_vars: bool = False + ) -> PROVIDER_TOKEN_TYPE | dict[str, str] | None: raise NotImplementedError() async def get_latest_token(self, provider_type: ProviderType) -> str | None: diff --git a/openhands/app_server/user/user_context.py b/openhands/app_server/user/user_context.py index 36aee854ae..625b4cd43e 100644 --- a/openhands/app_server/user/user_context.py +++ b/openhands/app_server/user/user_context.py @@ -35,8 +35,16 @@ class UserContext(ABC): """ @abstractmethod - async def get_provider_tokens(self) -> PROVIDER_TOKEN_TYPE | None: - """Get the latest tokens for all provider types""" + async def get_provider_tokens( + self, as_env_vars: bool = False + ) -> PROVIDER_TOKEN_TYPE | dict[str, str] | None: + """Get the latest tokens for all provider types. + + Args: + as_env_vars: When True, return a ``dict[str, str]`` mapping env + var names (e.g. ``github_token``) to plain-text token values. + When False (default), return the raw provider token mapping. + """ @abstractmethod async def get_latest_token(self, provider_type: ProviderType) -> str | None: diff --git a/openhands/app_server/user/user_router.py b/openhands/app_server/user/user_router.py index 2926c8495d..3cee936bde 100644 --- a/openhands/app_server/user/user_router.py +++ b/openhands/app_server/user/user_router.py @@ -1,12 +1,18 @@ """User router for OpenHands App Server. For the moment, this simply implements the /me endpoint.""" -from fastapi import APIRouter, HTTPException, status +import logging + +from fastapi import APIRouter, Header, HTTPException, Query, status +from fastapi.responses import JSONResponse from openhands.app_server.config import depends_user_context +from openhands.app_server.sandbox.session_auth import validate_session_key from openhands.app_server.user.user_context import UserContext from openhands.app_server.user.user_models import UserInfo from openhands.server.dependencies import get_dependencies +_logger = logging.getLogger(__name__) + # We use the get_dependencies method here to signal to the OpenAPI docs that this endpoint # is protected. The actual protection is provided by SetAuthCookieMiddleware router = APIRouter(prefix='/users', tags=['User'], dependencies=get_dependencies()) @@ -18,9 +24,52 @@ user_dependency = depends_user_context() @router.get('/me') async def get_current_user( user_context: UserContext = user_dependency, + expose_secrets: bool = Query( + default=False, + description='If true, return unmasked secret values (e.g. llm_api_key). ' + 'Requires a valid X-Session-API-Key header for an active sandbox ' + 'owned by the authenticated user.', + ), + x_session_api_key: str | None = Header(default=None), ) -> UserInfo: """Get the current authenticated user.""" user = await user_context.get_user_info() if user is None: raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail='Not authenticated') + if expose_secrets: + await _validate_session_key_ownership(user_context, x_session_api_key) + return JSONResponse( # type: ignore[return-value] + content=user.model_dump(mode='json', context={'expose_secrets': True}) + ) return user + + +async def _validate_session_key_ownership( + user_context: UserContext, + session_api_key: str | None, +) -> None: + """Verify the session key belongs to a sandbox owned by the caller. + + Raises ``HTTPException`` if the key is missing, invalid, or belongs + to a sandbox owned by a different user. + """ + sandbox_info = await validate_session_key(session_api_key) + + # Verify the sandbox is owned by the authenticated user. + caller_id = await user_context.get_user_id() + if not caller_id: + raise HTTPException( + status.HTTP_401_UNAUTHORIZED, + detail='Cannot determine authenticated user', + ) + + if sandbox_info.created_by_user_id != caller_id: + _logger.warning( + 'Session key user mismatch: sandbox owner=%s, caller=%s', + sandbox_info.created_by_user_id, + caller_id, + ) + raise HTTPException( + status.HTTP_403_FORBIDDEN, + detail='Session API key does not belong to the authenticated user', + ) diff --git a/tests/unit/app_server/test_sandbox_secrets_router.py b/tests/unit/app_server/test_sandbox_secrets_router.py new file mode 100644 index 0000000000..5976098bc1 --- /dev/null +++ b/tests/unit/app_server/test_sandbox_secrets_router.py @@ -0,0 +1,787 @@ +"""Unit + integration tests for the sandbox settings endpoints and /users/me expose_secrets. + +Tests: +- GET /api/v1/users/me?expose_secrets=true +- GET /api/v1/sandboxes/{sandbox_id}/settings/secrets +- GET /api/v1/sandboxes/{sandbox_id}/settings/secrets/{secret_name} +- Shared session_auth.validate_session_key() +- Integration tests exercising the real auth validation stack via HTTP +""" + +import contextlib +from unittest.mock import AsyncMock, patch + +import pytest +from fastapi import FastAPI, HTTPException +from fastapi.testclient import TestClient +from pydantic import SecretStr + +from openhands.app_server.sandbox.sandbox_models import ( + SandboxInfo, + SandboxStatus, + SecretNamesResponse, +) +from openhands.app_server.sandbox.sandbox_router import ( + get_secret_value, + list_secret_names, +) +from openhands.app_server.sandbox.session_auth import validate_session_key +from openhands.app_server.user.auth_user_context import AuthUserContext +from openhands.app_server.user.user_models import UserInfo +from openhands.app_server.user.user_router import ( + _validate_session_key_ownership, + get_current_user, +) +from openhands.integrations.provider import ProviderHandler, ProviderToken +from openhands.integrations.service_types import ProviderType +from openhands.sdk.secret import StaticSecret + +SANDBOX_ID = 'sb-test-123' +USER_ID = 'test-user-id' + + +def _make_sandbox_info( + sandbox_id: str = SANDBOX_ID, + user_id: str | None = USER_ID, +) -> SandboxInfo: + return SandboxInfo( + id=sandbox_id, + created_by_user_id=user_id, + sandbox_spec_id='test-spec', + status=SandboxStatus.RUNNING, + session_api_key='session-key', + ) + + +def _patch_sandbox_service(return_sandbox: SandboxInfo | None): + """Patch ``get_sandbox_service`` in ``session_auth`` to return a mock service.""" + mock_sandbox_service = AsyncMock() + mock_sandbox_service.get_sandbox_by_session_api_key = AsyncMock( + return_value=return_sandbox + ) + ctx = patch( + 'openhands.app_server.sandbox.session_auth.get_sandbox_service', + ) + return ctx, mock_sandbox_service + + +def _create_sandbox_service_context_manager(sandbox_service): + """Create an async context manager that yields the given sandbox service.""" + + @contextlib.asynccontextmanager + async def _context_manager(state, request=None): + yield sandbox_service + + return _context_manager + + +# --------------------------------------------------------------------------- +# validate_session_key (shared utility) +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestValidateSessionKey: + """Tests for the shared session_auth.validate_session_key utility.""" + + async def test_rejects_missing_key(self): + """Missing session key raises 401.""" + with pytest.raises(HTTPException) as exc_info: + await validate_session_key(None) + assert exc_info.value.status_code == 401 + assert 'X-Session-API-Key' in exc_info.value.detail + + async def test_rejects_empty_string_key(self): + """Empty string session key raises 401.""" + with pytest.raises(HTTPException) as exc_info: + await validate_session_key('') + assert exc_info.value.status_code == 401 + + async def test_rejects_invalid_key(self): + """Session key that maps to no sandbox raises 401.""" + ctx, mock_svc = _patch_sandbox_service(None) + with ctx as mock_get: + mock_get.return_value.__aenter__ = AsyncMock(return_value=mock_svc) + mock_get.return_value.__aexit__ = AsyncMock(return_value=False) + + with pytest.raises(HTTPException) as exc_info: + await validate_session_key('bogus-key') + assert exc_info.value.status_code == 401 + assert 'Invalid session API key' in exc_info.value.detail + + async def test_accepts_valid_key(self): + """Valid session key returns sandbox info.""" + sandbox = _make_sandbox_info() + ctx, mock_svc = _patch_sandbox_service(sandbox) + with ctx as mock_get: + mock_get.return_value.__aenter__ = AsyncMock(return_value=mock_svc) + mock_get.return_value.__aexit__ = AsyncMock(return_value=False) + + result = await validate_session_key('valid-key') + assert result.id == SANDBOX_ID + + async def test_rejects_sandbox_without_user_in_saas_mode(self): + """In SAAS mode, sandbox without created_by_user_id raises 401.""" + sandbox = _make_sandbox_info(user_id=None) + ctx, mock_svc = _patch_sandbox_service(sandbox) + with ( + ctx as mock_get, + patch( + 'openhands.app_server.sandbox.session_auth.get_global_config' + ) as mock_cfg, + ): + mock_get.return_value.__aenter__ = AsyncMock(return_value=mock_svc) + mock_get.return_value.__aexit__ = AsyncMock(return_value=False) + + from openhands.server.types import AppMode + + mock_cfg.return_value.app_mode = AppMode.SAAS + + with pytest.raises(HTTPException) as exc_info: + await validate_session_key('valid-key') + assert exc_info.value.status_code == 401 + assert 'no user' in exc_info.value.detail + + +# --------------------------------------------------------------------------- +# GET /users/me?expose_secrets=true +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestGetCurrentUserExposeSecrets: + """Test suite for GET /users/me?expose_secrets=true.""" + + async def test_expose_secrets_returns_raw_api_key(self): + """With valid session key, expose_secrets=true returns unmasked llm_api_key.""" + user_info = UserInfo( + id=USER_ID, + llm_model='anthropic/claude-sonnet-4-20250514', + llm_api_key=SecretStr('sk-test-key-123'), + llm_base_url='https://litellm.example.com', + ) + mock_context = AsyncMock() + mock_context.get_user_info = AsyncMock(return_value=user_info) + mock_context.get_user_id = AsyncMock(return_value=USER_ID) + + with patch( + 'openhands.app_server.user.user_router._validate_session_key_ownership' + ) as mock_validate: + mock_validate.return_value = None + result = await get_current_user( + user_context=mock_context, + expose_secrets=True, + x_session_api_key='valid-key', + ) + + # JSONResponse — parse the body + import json + + body = json.loads(result.body) + assert body['llm_model'] == 'anthropic/claude-sonnet-4-20250514' + assert body['llm_api_key'] == 'sk-test-key-123' + assert body['llm_base_url'] == 'https://litellm.example.com' + + async def test_expose_secrets_rejects_missing_session_key(self): + """expose_secrets=true without X-Session-API-Key is rejected.""" + mock_context = AsyncMock() + + with pytest.raises(HTTPException) as exc_info: + await _validate_session_key_ownership(mock_context, session_api_key=None) + assert exc_info.value.status_code == 401 + assert 'X-Session-API-Key' in exc_info.value.detail + + async def test_expose_secrets_rejects_wrong_user(self): + """expose_secrets=true with session key from different user is rejected.""" + mock_context = AsyncMock() + mock_context.get_user_id = AsyncMock(return_value='user-A') + + other_user_sandbox = _make_sandbox_info(user_id='user-B') + + ctx, mock_svc = _patch_sandbox_service(other_user_sandbox) + with ctx as mock_get, pytest.raises(HTTPException) as exc_info: + mock_get.return_value.__aenter__ = AsyncMock(return_value=mock_svc) + mock_get.return_value.__aexit__ = AsyncMock(return_value=False) + + await _validate_session_key_ownership( + mock_context, session_api_key='stolen-key' + ) + + assert exc_info.value.status_code == 403 + + async def test_expose_secrets_rejects_unknown_caller(self): + """If caller_id cannot be determined, reject with 401.""" + mock_context = AsyncMock() + mock_context.get_user_id = AsyncMock(return_value=None) + + sandbox = _make_sandbox_info(user_id='user-B') + + ctx, mock_svc = _patch_sandbox_service(sandbox) + with ctx as mock_get, pytest.raises(HTTPException) as exc_info: + mock_get.return_value.__aenter__ = AsyncMock(return_value=mock_svc) + mock_get.return_value.__aexit__ = AsyncMock(return_value=False) + + await _validate_session_key_ownership( + mock_context, session_api_key='some-key' + ) + + assert exc_info.value.status_code == 401 + assert 'Cannot determine authenticated user' in exc_info.value.detail + + async def test_default_masks_api_key(self): + """Without expose_secrets, llm_api_key is masked (no session key needed).""" + user_info = UserInfo( + id=USER_ID, + llm_api_key=SecretStr('sk-test-key-123'), + ) + mock_context = AsyncMock() + mock_context.get_user_info = AsyncMock(return_value=user_info) + + result = await get_current_user( + user_context=mock_context, expose_secrets=False, x_session_api_key=None + ) + + # Returns UserInfo directly (FastAPI will serialize with masking) + assert isinstance(result, UserInfo) + assert result.llm_api_key is not None + # The raw value is still in the object, but serialization masks it + dumped = result.model_dump(mode='json') + assert dumped['llm_api_key'] == '**********' + + +# --------------------------------------------------------------------------- +# GET /sandboxes/{sandbox_id}/settings/secrets +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestListSecretNames: + """Test suite for GET /sandboxes/{sandbox_id}/settings/secrets.""" + + async def test_returns_secret_names_without_values(self): + """Response contains names and descriptions, NOT raw values.""" + secrets = { + 'GITHUB_TOKEN': StaticSecret( + value=SecretStr('ghp_test123'), + description='GitHub personal access token', + ), + 'MY_API_KEY': StaticSecret( + value=SecretStr('my-api-key-value'), + description='Custom API key', + ), + } + sandbox_info = _make_sandbox_info() + + with patch( + 'openhands.app_server.sandbox.sandbox_router._get_user_context' + ) as mock_ctx: + ctx = AsyncMock() + ctx.get_secrets = AsyncMock(return_value=secrets) + ctx.get_provider_tokens = AsyncMock(return_value={}) + mock_ctx.return_value = ctx + + result = await list_secret_names(sandbox_info=sandbox_info) + + assert isinstance(result, SecretNamesResponse) + assert len(result.secrets) == 2 + names = {s.name for s in result.secrets} + assert 'GITHUB_TOKEN' in names + assert 'MY_API_KEY' in names + + gh = next(s for s in result.secrets if s.name == 'GITHUB_TOKEN') + assert gh.description == 'GitHub personal access token' + # Verify no 'value' field is exposed + assert not hasattr(gh, 'value') + + async def test_returns_empty_when_no_secrets(self): + sandbox_info = _make_sandbox_info() + + with patch( + 'openhands.app_server.sandbox.sandbox_router._get_user_context' + ) as mock_ctx: + ctx = AsyncMock() + ctx.get_secrets = AsyncMock(return_value={}) + ctx.get_provider_tokens = AsyncMock(return_value={}) + mock_ctx.return_value = ctx + + result = await list_secret_names(sandbox_info=sandbox_info) + + assert len(result.secrets) == 0 + + +# --------------------------------------------------------------------------- +# GET /sandboxes/{sandbox_id}/settings/secrets/{name} +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestGetSecretValue: + """Test suite for GET /sandboxes/{sandbox_id}/settings/secrets/{name}.""" + + async def test_returns_raw_secret_value(self): + """Raw secret value returned as plain text.""" + secrets = { + 'GITHUB_TOKEN': StaticSecret( + value=SecretStr('ghp_actual_secret'), + description='GitHub token', + ), + } + sandbox_info = _make_sandbox_info() + + with patch( + 'openhands.app_server.sandbox.sandbox_router._get_user_context' + ) as mock_ctx: + ctx = AsyncMock() + ctx.get_secrets = AsyncMock(return_value=secrets) + ctx.get_provider_tokens = AsyncMock(return_value={}) + mock_ctx.return_value = ctx + + response = await get_secret_value( + secret_name='GITHUB_TOKEN', + sandbox_info=sandbox_info, + ) + + assert response.body == b'ghp_actual_secret' + assert response.media_type == 'text/plain' + + async def test_returns_404_for_unknown_secret(self): + """404 when requested secret doesn't exist in custom secrets or provider tokens.""" + sandbox_info = _make_sandbox_info() + + with patch( + 'openhands.app_server.sandbox.sandbox_router._get_user_context' + ) as mock_ctx: + ctx = AsyncMock() + ctx.get_secrets = AsyncMock(return_value={}) + ctx.get_provider_tokens = AsyncMock(return_value={}) + mock_ctx.return_value = ctx + + with pytest.raises(HTTPException) as exc_info: + await get_secret_value( + secret_name='NONEXISTENT', + sandbox_info=sandbox_info, + ) + + assert exc_info.value.status_code == 404 + + async def test_returns_404_for_none_value_secret(self): + """404 when secret exists but has None value.""" + secrets = { + 'EMPTY_SECRET': StaticSecret(value=None), + } + sandbox_info = _make_sandbox_info() + + with patch( + 'openhands.app_server.sandbox.sandbox_router._get_user_context' + ) as mock_ctx: + ctx = AsyncMock() + ctx.get_secrets = AsyncMock(return_value=secrets) + ctx.get_provider_tokens = AsyncMock(return_value={}) + mock_ctx.return_value = ctx + + with pytest.raises(HTTPException) as exc_info: + await get_secret_value( + secret_name='EMPTY_SECRET', + sandbox_info=sandbox_info, + ) + + assert exc_info.value.status_code == 404 + + +# =========================================================================== +# Integration tests — real HTTP requests through real auth validation logic. +# +# Only the data layer (sandbox service, user context) is mocked. +# The session key validation, ownership checks, and FastAPI routing are REAL. +# =========================================================================== + + +def _build_integration_test_app( + mock_user_context: AsyncMock | None = None, +) -> FastAPI: + """Build a minimal FastAPI app with the real user and sandbox routers. + + The ``depends_user_context`` dependency is overridden with a mock, but the + session key validation logic in ``validate_session_key`` and + ``_validate_session_key_ownership`` runs unmodified. + + Router-level dependencies (e.g. ``check_session_api_key`` from ``SESSION_API_KEY`` + env var) are overridden to no-ops so we can exercise the endpoint-level auth logic + in isolation. + """ + from openhands.app_server.sandbox.sandbox_router import ( + router as sandbox_router, + ) + from openhands.app_server.user.user_router import router as user_router + from openhands.server.dependencies import check_session_api_key + + app = FastAPI() + + # Disable router-level auth (SESSION_API_KEY check) — we're testing the + # endpoint-level session key validation, not the router middleware. + app.dependency_overrides[check_session_api_key] = lambda: None + + if mock_user_context is not None: + from openhands.app_server.user.user_router import user_dependency + + app.dependency_overrides[user_dependency.dependency] = lambda: mock_user_context + + app.include_router(user_router, prefix='/api/v1') + app.include_router(sandbox_router, prefix='/api/v1') + return app + + +class TestExposeSecretsIntegration: + """Integration tests for /users/me?expose_secrets=true via real HTTP. + + These tests exercise the full auth validation stack: + - validate_session_key (real) + - _validate_session_key_ownership (real) + - ownership check (real) + Only the data layer (sandbox service lookup, user context) is mocked. + """ + + def test_expose_secrets_without_session_key_returns_401(self): + """Bearer token alone cannot expose secrets (no X-Session-API-Key).""" + mock_user_ctx = AsyncMock() + mock_user_ctx.get_user_info = AsyncMock( + return_value=UserInfo(id=USER_ID, llm_api_key=SecretStr('sk-secret-123')) + ) + mock_user_ctx.get_user_id = AsyncMock(return_value=USER_ID) + + app = _build_integration_test_app(mock_user_ctx) + client = TestClient(app, raise_server_exceptions=False) + + response = client.get('/api/v1/users/me', params={'expose_secrets': 'true'}) + + assert response.status_code == 401 + assert 'X-Session-API-Key' in response.json()['detail'] + + def test_expose_secrets_with_invalid_session_key_returns_401(self): + """Invalid session key (no matching sandbox) is rejected.""" + mock_user_ctx = AsyncMock() + mock_user_ctx.get_user_info = AsyncMock( + return_value=UserInfo(id=USER_ID, llm_api_key=SecretStr('sk-secret-123')) + ) + mock_user_ctx.get_user_id = AsyncMock(return_value=USER_ID) + + mock_sandbox_svc = AsyncMock() + mock_sandbox_svc.get_sandbox_by_session_api_key = AsyncMock(return_value=None) + + app = _build_integration_test_app(mock_user_ctx) + client = TestClient(app, raise_server_exceptions=False) + + with patch( + 'openhands.app_server.sandbox.session_auth.get_sandbox_service', + _create_sandbox_service_context_manager(mock_sandbox_svc), + ): + response = client.get( + '/api/v1/users/me', + params={'expose_secrets': 'true'}, + headers={'X-Session-API-Key': 'bogus-key'}, + ) + + assert response.status_code == 401 + assert 'Invalid session API key' in response.json()['detail'] + + def test_expose_secrets_with_wrong_user_returns_403(self): + """Session key from a different user's sandbox is rejected.""" + mock_user_ctx = AsyncMock() + mock_user_ctx.get_user_info = AsyncMock( + return_value=UserInfo(id='user-A', llm_api_key=SecretStr('sk-secret-123')) + ) + mock_user_ctx.get_user_id = AsyncMock(return_value='user-A') + + # Sandbox owned by user-B + sandbox_b = _make_sandbox_info(user_id='user-B') + mock_sandbox_svc = AsyncMock() + mock_sandbox_svc.get_sandbox_by_session_api_key = AsyncMock( + return_value=sandbox_b + ) + + app = _build_integration_test_app(mock_user_ctx) + client = TestClient(app, raise_server_exceptions=False) + + with patch( + 'openhands.app_server.sandbox.session_auth.get_sandbox_service', + _create_sandbox_service_context_manager(mock_sandbox_svc), + ): + response = client.get( + '/api/v1/users/me', + params={'expose_secrets': 'true'}, + headers={'X-Session-API-Key': 'stolen-key'}, + ) + + assert response.status_code == 403 + assert 'does not belong' in response.json()['detail'] + + def test_expose_secrets_valid_dual_auth_returns_200_unmasked(self): + """Valid Bearer + valid session key owned by same user → 200 with secrets.""" + mock_user_ctx = AsyncMock() + mock_user_ctx.get_user_info = AsyncMock( + return_value=UserInfo( + id=USER_ID, + llm_model='anthropic/claude-sonnet-4-20250514', + llm_api_key=SecretStr('sk-real-secret'), + llm_base_url='https://litellm.example.com', + ) + ) + mock_user_ctx.get_user_id = AsyncMock(return_value=USER_ID) + + sandbox = _make_sandbox_info(user_id=USER_ID) + mock_sandbox_svc = AsyncMock() + mock_sandbox_svc.get_sandbox_by_session_api_key = AsyncMock( + return_value=sandbox + ) + + app = _build_integration_test_app(mock_user_ctx) + client = TestClient(app, raise_server_exceptions=False) + + with patch( + 'openhands.app_server.sandbox.session_auth.get_sandbox_service', + _create_sandbox_service_context_manager(mock_sandbox_svc), + ): + response = client.get( + '/api/v1/users/me', + params={'expose_secrets': 'true'}, + headers={'X-Session-API-Key': 'valid-key'}, + ) + + assert response.status_code == 200 + body = response.json() + assert body['llm_api_key'] == 'sk-real-secret' + assert body['llm_model'] == 'anthropic/claude-sonnet-4-20250514' + assert body['llm_base_url'] == 'https://litellm.example.com' + + def test_default_masks_secrets_via_http(self): + """Without expose_secrets, secrets are masked even via real HTTP.""" + mock_user_ctx = AsyncMock() + mock_user_ctx.get_user_info = AsyncMock( + return_value=UserInfo( + id=USER_ID, llm_api_key=SecretStr('sk-should-be-masked') + ) + ) + + app = _build_integration_test_app(mock_user_ctx) + client = TestClient(app, raise_server_exceptions=False) + + response = client.get('/api/v1/users/me') + + assert response.status_code == 200 + body = response.json() + assert body['llm_api_key'] == '**********' + + +class TestSandboxSecretsIntegration: + """Integration tests for sandbox-scoped secrets endpoints via real HTTP. + + The session key validation in ``_valid_sandbox_from_session_key`` runs + unmodified — only the sandbox service (database) is mocked. + """ + + def test_secrets_list_without_session_key_returns_401(self): + """Missing X-Session-API-Key on secrets endpoint is rejected.""" + app = _build_integration_test_app() + client = TestClient(app, raise_server_exceptions=False) + + response = client.get(f'/api/v1/sandboxes/{SANDBOX_ID}/settings/secrets') + + assert response.status_code == 401 + assert 'X-Session-API-Key' in response.json()['detail'] + + def test_secrets_list_with_invalid_session_key_returns_401(self): + """Invalid session key on secrets endpoint is rejected.""" + app = _build_integration_test_app() + client = TestClient(app, raise_server_exceptions=False) + + mock_sandbox_svc = AsyncMock() + mock_sandbox_svc.get_sandbox_by_session_api_key = AsyncMock(return_value=None) + + with patch( + 'openhands.app_server.sandbox.session_auth.get_sandbox_service', + _create_sandbox_service_context_manager(mock_sandbox_svc), + ): + response = client.get( + f'/api/v1/sandboxes/{SANDBOX_ID}/settings/secrets', + headers={'X-Session-API-Key': 'bogus'}, + ) + + assert response.status_code == 401 + assert 'Invalid session API key' in response.json()['detail'] + + def test_secrets_list_with_mismatched_sandbox_id_returns_403(self): + """Session key maps to a different sandbox than the URL path → 403.""" + app = _build_integration_test_app() + client = TestClient(app, raise_server_exceptions=False) + + # Session key maps to sandbox "other-sandbox", but URL says SANDBOX_ID + other_sandbox = _make_sandbox_info(sandbox_id='other-sandbox') + mock_sandbox_svc = AsyncMock() + mock_sandbox_svc.get_sandbox_by_session_api_key = AsyncMock( + return_value=other_sandbox + ) + + with patch( + 'openhands.app_server.sandbox.session_auth.get_sandbox_service', + _create_sandbox_service_context_manager(mock_sandbox_svc), + ): + response = client.get( + f'/api/v1/sandboxes/{SANDBOX_ID}/settings/secrets', + headers={'X-Session-API-Key': 'valid-key'}, + ) + + assert response.status_code == 403 + assert 'does not match' in response.json()['detail'] + + def test_sandbox_without_user_returns_401_for_secret_value(self): + """Sandbox with no owning user → 401 when fetching a secret value.""" + app = _build_integration_test_app() + client = TestClient(app, raise_server_exceptions=False) + + # Sandbox exists but has no owning user + sandbox_no_user = _make_sandbox_info(user_id=None) + mock_sandbox_svc = AsyncMock() + mock_sandbox_svc.get_sandbox_by_session_api_key = AsyncMock( + return_value=sandbox_no_user + ) + + with patch( + 'openhands.app_server.sandbox.session_auth.get_sandbox_service', + _create_sandbox_service_context_manager(mock_sandbox_svc), + ): + response = client.get( + f'/api/v1/sandboxes/{SANDBOX_ID}/settings/secrets/MY_SECRET', + headers={'X-Session-API-Key': 'valid-key'}, + ) + + # _get_user_context raises 401 because created_by_user_id is None + assert response.status_code == 401 + assert 'no associated user' in response.json()['detail'] + + +# --------------------------------------------------------------------------- +# Provider tokens in sandbox secrets endpoints +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +class TestProviderTokensInEndpoints: + """Verify that sandbox secrets endpoints include provider tokens resolved lazily.""" + + async def test_get_provider_tokens_as_env_vars(self): + """get_provider_tokens(as_env_vars=True) returns fresh values keyed by env name.""" + mock_user_auth = AsyncMock() + mock_user_auth.get_provider_tokens = AsyncMock( + return_value={ + ProviderType.GITHUB: ProviderToken(token=SecretStr('ghp_test123')), + ProviderType.GITLAB: ProviderToken(token=SecretStr('glpat-test456')), + } + ) + + ctx = AuthUserContext(user_auth=mock_user_auth) + result = await ctx.get_provider_tokens(as_env_vars=True) + + gh_key = ProviderHandler.get_provider_env_key(ProviderType.GITHUB) + gl_key = ProviderHandler.get_provider_env_key(ProviderType.GITLAB) + assert result[gh_key] == 'ghp_test123' + assert result[gl_key] == 'glpat-test456' + + async def test_empty_provider_tokens_excluded(self): + """Provider tokens with empty token values are excluded.""" + mock_user_auth = AsyncMock() + mock_user_auth.get_provider_tokens = AsyncMock( + return_value={ + ProviderType.GITHUB: ProviderToken(token=SecretStr('')), + } + ) + + ctx = AuthUserContext(user_auth=mock_user_auth) + result = await ctx.get_provider_tokens(as_env_vars=True) + + gh_key = ProviderHandler.get_provider_env_key(ProviderType.GITHUB) + assert gh_key not in result + + async def test_none_provider_tokens_returns_empty(self): + """get_provider_tokens(as_env_vars=True) with None tokens yields empty dict.""" + mock_user_auth = AsyncMock() + mock_user_auth.get_provider_tokens = AsyncMock(return_value=None) + + ctx = AuthUserContext(user_auth=mock_user_auth) + result = await ctx.get_provider_tokens(as_env_vars=True) + assert result == {} + + async def test_list_secret_names_includes_provider_tokens(self): + """list_secret_names returns both custom secrets and provider token names.""" + custom_secrets = { + 'MY_KEY': StaticSecret( + value=SecretStr('my-value'), description='custom key' + ), + } + gh_key = ProviderHandler.get_provider_env_key(ProviderType.GITHUB) + provider_env_vars = {gh_key: 'ghp_test123'} + + sandbox_info = _make_sandbox_info() + + with patch( + 'openhands.app_server.sandbox.sandbox_router._get_user_context' + ) as mock_ctx: + ctx = AsyncMock() + ctx.get_secrets = AsyncMock(return_value=custom_secrets) + ctx.get_provider_tokens = AsyncMock(return_value=provider_env_vars) + mock_ctx.return_value = ctx + + result = await list_secret_names(sandbox_info=sandbox_info) + + names = {s.name for s in result.secrets} + assert 'MY_KEY' in names + assert gh_key in names + assert len(result.secrets) == 2 + + async def test_get_secret_value_resolves_provider_token(self): + """get_secret_value falls back to provider tokens when not in custom secrets.""" + gh_key = ProviderHandler.get_provider_env_key(ProviderType.GITHUB) + sandbox_info = _make_sandbox_info() + + with patch( + 'openhands.app_server.sandbox.sandbox_router._get_user_context' + ) as mock_ctx: + ctx = AsyncMock() + ctx.get_secrets = AsyncMock(return_value={}) + ctx.get_provider_tokens = AsyncMock( + return_value={gh_key: 'ghp_fresh_token'} + ) + mock_ctx.return_value = ctx + + response = await get_secret_value( + secret_name=gh_key, sandbox_info=sandbox_info + ) + + assert response.body == b'ghp_fresh_token' + assert response.media_type == 'text/plain' + + async def test_custom_secret_takes_priority_over_provider_token(self): + """If a custom secret has the same name, it takes priority.""" + gh_key = ProviderHandler.get_provider_env_key(ProviderType.GITHUB) + sandbox_info = _make_sandbox_info() + + with patch( + 'openhands.app_server.sandbox.sandbox_router._get_user_context' + ) as mock_ctx: + ctx = AsyncMock() + ctx.get_secrets = AsyncMock( + return_value={ + gh_key: StaticSecret( + value=SecretStr('custom-override'), + description='user override', + ) + } + ) + # Provider token should NOT be called since custom secret matches + ctx.get_provider_tokens = AsyncMock(return_value={gh_key: 'provider-value'}) + mock_ctx.return_value = ctx + + response = await get_secret_value( + secret_name=gh_key, sandbox_info=sandbox_info + ) + + assert response.body == b'custom-override'