mirror of
https://github.com/OpenHands/OpenHands.git
synced 2026-03-22 05:37:20 +08:00
feat: expose_secrets param on /users/me + sandbox-scoped secrets API (#13383)
Co-authored-by: openhands <openhands@all-hands.dev>
This commit is contained in:
27
AGENTS.md
27
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
|
- Models appear in CLI provider selection based on the verified arrays
|
||||||
- The `organize_models_and_providers` function groups models by provider
|
- The `organize_models_and_providers` function groups models by provider
|
||||||
- Default model selection prioritizes verified models for each 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`
|
||||||
|
|||||||
@@ -60,7 +60,9 @@ class ResolverUserContext(UserContext):
|
|||||||
return provider_token.token.get_secret_value()
|
return provider_token.token.get_secret_value()
|
||||||
return None
|
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()
|
return await self.saas_user_auth.get_provider_tokens()
|
||||||
|
|
||||||
async def get_secrets(self) -> dict[str, SecretSource]:
|
async def get_secrets(self) -> dict[str, SecretSource]:
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ import zipfile
|
|||||||
from collections import defaultdict
|
from collections import defaultdict
|
||||||
from dataclasses import dataclass
|
from dataclasses import dataclass
|
||||||
from datetime import datetime, timedelta
|
from datetime import datetime, timedelta
|
||||||
from typing import Any, AsyncGenerator, Sequence
|
from typing import Any, AsyncGenerator, Sequence, cast
|
||||||
from uuid import UUID, uuid4
|
from uuid import UUID, uuid4
|
||||||
|
|
||||||
import httpx
|
import httpx
|
||||||
@@ -84,7 +84,7 @@ from openhands.app_server.utils.llm_metadata import (
|
|||||||
get_llm_metadata,
|
get_llm_metadata,
|
||||||
should_set_litellm_extra_body,
|
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.integrations.service_types import SuggestedTask
|
||||||
from openhands.sdk import Agent, AgentContext, LocalWorkspace
|
from openhands.sdk import Agent, AgentContext, LocalWorkspace
|
||||||
from openhands.sdk.hooks import HookConfig
|
from openhands.sdk.hooks import HookConfig
|
||||||
@@ -837,7 +837,10 @@ class LiveStatusAppConversationService(AppConversationServiceBase):
|
|||||||
secrets = await self.user_context.get_secrets()
|
secrets = await self.user_context.get_secrets()
|
||||||
|
|
||||||
# Get all provider tokens from user authentication
|
# 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:
|
if not provider_tokens:
|
||||||
return secrets
|
return secrets
|
||||||
|
|
||||||
|
|||||||
@@ -59,3 +59,20 @@ class SandboxInfo(BaseModel):
|
|||||||
class SandboxPage(BaseModel):
|
class SandboxPage(BaseModel):
|
||||||
items: list[SandboxInfo]
|
items: list[SandboxInfo]
|
||||||
next_page_id: str | None = None
|
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'
|
||||||
|
)
|
||||||
|
|||||||
@@ -1,16 +1,30 @@
|
|||||||
"""Runtime Containers router for OpenHands App Server."""
|
"""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.agent_server.models import Success
|
||||||
from openhands.app_server.config import depends_sandbox_service
|
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 (
|
from openhands.app_server.sandbox.sandbox_service import (
|
||||||
SandboxService,
|
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.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
|
# 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
|
# is protected. The actual protection is provided by SetAuthCookieMiddleware
|
||||||
@@ -94,3 +108,104 @@ async def delete_sandbox(
|
|||||||
if not exists:
|
if not exists:
|
||||||
raise HTTPException(status.HTTP_404_NOT_FOUND)
|
raise HTTPException(status.HTTP_404_NOT_FOUND)
|
||||||
return Success()
|
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')
|
||||||
|
|||||||
66
openhands/app_server/sandbox/session_auth.py
Normal file
66
openhands/app_server/sandbox/session_auth.py
Normal file
@@ -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
|
||||||
@@ -48,8 +48,27 @@ class AuthUserContext(UserContext):
|
|||||||
self._user_info = user_info
|
self._user_info = user_info
|
||||||
return user_info
|
return user_info
|
||||||
|
|
||||||
async def get_provider_tokens(self) -> PROVIDER_TOKEN_TYPE | None:
|
async def get_provider_tokens(
|
||||||
return await self.user_auth.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):
|
async def get_provider_handler(self):
|
||||||
provider_handler = self._provider_handler
|
provider_handler = self._provider_handler
|
||||||
@@ -79,9 +98,9 @@ class AuthUserContext(UserContext):
|
|||||||
return token
|
return token
|
||||||
|
|
||||||
async def get_secrets(self) -> dict[str, SecretSource]:
|
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()
|
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():
|
||||||
|
|||||||
@@ -26,7 +26,9 @@ class SpecifyUserContext(UserContext):
|
|||||||
) -> str:
|
) -> str:
|
||||||
raise NotImplementedError()
|
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()
|
raise NotImplementedError()
|
||||||
|
|
||||||
async def get_latest_token(self, provider_type: ProviderType) -> str | None:
|
async def get_latest_token(self, provider_type: ProviderType) -> str | None:
|
||||||
|
|||||||
@@ -35,8 +35,16 @@ class UserContext(ABC):
|
|||||||
"""
|
"""
|
||||||
|
|
||||||
@abstractmethod
|
@abstractmethod
|
||||||
async def get_provider_tokens(self) -> PROVIDER_TOKEN_TYPE | None:
|
async def get_provider_tokens(
|
||||||
"""Get the latest tokens for all provider types"""
|
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
|
@abstractmethod
|
||||||
async def get_latest_token(self, provider_type: ProviderType) -> str | None:
|
async def get_latest_token(self, provider_type: ProviderType) -> str | None:
|
||||||
|
|||||||
@@ -1,12 +1,18 @@
|
|||||||
"""User router for OpenHands App Server. For the moment, this simply implements the /me endpoint."""
|
"""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.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_context import UserContext
|
||||||
from openhands.app_server.user.user_models import UserInfo
|
from openhands.app_server.user.user_models import UserInfo
|
||||||
from openhands.server.dependencies import get_dependencies
|
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
|
# 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
|
# is protected. The actual protection is provided by SetAuthCookieMiddleware
|
||||||
router = APIRouter(prefix='/users', tags=['User'], dependencies=get_dependencies())
|
router = APIRouter(prefix='/users', tags=['User'], dependencies=get_dependencies())
|
||||||
@@ -18,9 +24,52 @@ user_dependency = depends_user_context()
|
|||||||
@router.get('/me')
|
@router.get('/me')
|
||||||
async def get_current_user(
|
async def get_current_user(
|
||||||
user_context: UserContext = user_dependency,
|
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:
|
) -> UserInfo:
|
||||||
"""Get the current authenticated user."""
|
"""Get the current authenticated user."""
|
||||||
user = await user_context.get_user_info()
|
user = await user_context.get_user_info()
|
||||||
if user is None:
|
if user is None:
|
||||||
raise HTTPException(status.HTTP_401_UNAUTHORIZED, detail='Not authenticated')
|
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
|
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',
|
||||||
|
)
|
||||||
|
|||||||
787
tests/unit/app_server/test_sandbox_secrets_router.py
Normal file
787
tests/unit/app_server/test_sandbox_secrets_router.py
Normal file
@@ -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'
|
||||||
Reference in New Issue
Block a user