Compare commits

...

7 Commits

Author SHA1 Message Date
Chuck Butkus 8197582aa4 Add ruff error 2025-10-01 17:00:49 -04:00
openhands 1d8d5981e8 Add verbose output to ruff pre-commit hooks
- Add --verbose flag to ruff linter in main pre-commit config
- Add --verbose flag to ruff linter in enterprise pre-commit config
- This will provide detailed debug information and error context
  when ruff finds linting issues in GitHub Actions

Co-authored-by: openhands <openhands@all-hands.dev>
2025-10-01 17:04:32 +00:00
Hiep Le a8b6406dac fix: searching repositories (#11203) 2025-10-01 22:21:20 +07:00
Hiep Le 509d4a9513 fix(frontend): the horizontal scrollbar is not displayed for the markdown content (#11200) 2025-10-01 12:31:56 +07:00
Alona d099c21f5d fix: add provider tokens to resume conversation endpoint (#11155) 2025-10-01 05:20:48 +07:00
Hiep Le 4c89b5ad91 fix: showing unrelated files in the changes tab (#11185)
Co-authored-by: Graham Neubig <neubig@gmail.com>
2025-10-01 01:44:36 +07:00
sp.wack 729c181313 Revert "fix(backend): Add validation for LLM settings to prevent non-pro user bypass" (#11192) 2025-09-30 17:54:33 +00:00
19 changed files with 400 additions and 505 deletions
+1 -1
View File
@@ -27,7 +27,7 @@ repos:
- id: ruff
entry: ruff check --config dev_config/python/ruff.toml
types_or: [python, pyi, jupyter]
args: [--fix, --unsafe-fixes]
args: [--fix, --unsafe-fixes, --verbose]
exclude: ^(third_party/|enterprise/)
# Run the formatter.
- id: ruff-format
@@ -27,7 +27,7 @@ repos:
- id: ruff
entry: ruff check --config enterprise/dev_config/python/ruff.toml
types_or: [python, pyi, jupyter]
args: [--fix]
args: [--fix, --verbose]
files: ^enterprise/
# Run the formatter.
- id: ruff-format
@@ -5,12 +5,6 @@ from dataclasses import dataclass, field
from uuid import uuid4
import socketio
from server.logger import logger
from server.utils.conversation_callback_utils import invoke_conversation_callbacks
from storage.database import session_maker
from storage.saas_settings_store import SaasSettingsStore
from storage.stored_conversation_metadata import StoredConversationMetadata
from openhands.core.config import LLMConfig
from openhands.core.config.openhands_config import OpenHandsConfig
from openhands.core.config.utils import load_openhands_config
@@ -37,6 +31,12 @@ from openhands.storage.files import FileStore
from openhands.utils.async_utils import call_sync_from_async, wait_all
from openhands.utils.shutdown_listener import should_continue
from server.logger import logger
from server.utils.conversation_callback_utils import invoke_conversation_callbacks
from storage.database import session_maker
from storage.saas_settings_store import SaasSettingsStore
from storage.stored_conversation_metadata import StoredConversationMetadata
# Time in seconds between cleanup operations for stale conversations
_CLEANUP_INTERVAL_SECONDS = 15
+2
View File
@@ -138,6 +138,7 @@ async def saas_search_repositories(
per_page: int = 5,
sort: str = 'stars',
order: str = 'desc',
selected_provider: ProviderType | None = None,
provider_tokens: PROVIDER_TOKEN_TYPE | None = Depends(get_provider_tokens),
access_token: SecretStr | None = Depends(get_access_token),
user_id: str | None = Depends(get_user_id),
@@ -155,6 +156,7 @@ async def saas_search_repositories(
per_page=per_page,
sort=sort,
order=order,
selected_provider=selected_provider,
provider_tokens=provider_tokens,
access_token=access_token,
user_id=user_id,
@@ -23,7 +23,7 @@ class GitService {
*/
static async searchGitRepositories(
query: string,
per_page = 5,
per_page = 100,
selected_provider?: Provider,
): Promise<GitRepository[]> {
const response = await openHands.get<GitRepository[]>(
@@ -107,7 +107,7 @@ export function ChatMessage({
</div>
<div
className="text-sm w-fit"
className="text-sm"
style={{
whiteSpace: "normal",
wordBreak: "break-word",
@@ -6,7 +6,7 @@ export function useSearchRepositories(
query: string,
selectedProvider?: Provider | null,
disabled?: boolean,
pageSize: number = 3,
pageSize: number = 100,
) {
return useQuery({
queryKey: ["repositories", "search", query, selectedProvider, pageSize],
+9 -7
View File
@@ -88,13 +88,15 @@ function GitChanges() {
</div>
</div>
) : (
gitChanges.map((change) => (
<FileDiffViewer
key={change.path}
path={change.path}
type={change.status}
/>
))
gitChanges
.slice(0, 100)
.map((change) => (
<FileDiffViewer
key={change.path}
path={change.path}
type={change.status}
/>
))
)}
</main>
);
@@ -28,6 +28,7 @@ Your primary role is to assist users by executing commands, modifying code, and
* Before implementing any changes, first thoroughly understand the codebase through exploration.
* If you are adding a lot of code to a function or file, consider splitting the function or file into smaller pieces when appropriate.
* Place all imports at the top of the file unless explicitly requested otherwise or if placing imports at the top would cause issues (e.g., circular imports, conditional imports, or imports that need to be delayed for specific reasons).
* If working in a git repo, before you commit code create a .gitignore file if one doesn't exist. And if there are existing files that should not be included then update the .gitignore file as appropriate.
</CODE_QUALITY>
<VERSION_CONTROL>
@@ -214,7 +214,7 @@ class GitHubReposMixin(GitHubMixinBase):
all_repos = []
# Search in user repositories
user_query = f'{query} user:{user.login}'
user_query = f'in:name {query} user:{user.login}'
user_params = params.copy()
user_params['q'] = user_query
+16 -4
View File
@@ -466,10 +466,22 @@ class ProviderHandler:
except Exception as e:
errors.append(f'{provider.value}: {str(e)}')
# Log all accumulated errors before raising AuthenticationError
logger.error(
f'Failed to access repository {repository} with all available providers. Errors: {"; ".join(errors)}'
)
# Log detailed error based on whether we had tokens or not
if not self.provider_tokens:
logger.error(
f'Failed to access repository {repository}: No provider tokens available. '
f'provider_tokens dict is empty.'
)
elif errors:
logger.error(
f'Failed to access repository {repository} with all available providers. '
f'Tried providers: {list(self.provider_tokens.keys())}. '
f'Errors: {"; ".join(errors)}'
)
else:
logger.error(
f'Failed to access repository {repository}: Unknown error (no providers tried, no errors recorded)'
)
raise AuthenticationError(f'Unable to access repo {repository}')
async def get_branches(
@@ -462,6 +462,7 @@ async def start_conversation(
providers_set: ProvidersSetModel,
conversation_id: str = Depends(validate_conversation_id),
user_id: str = Depends(get_user_id),
provider_tokens: PROVIDER_TOKEN_TYPE = Depends(get_provider_tokens),
settings: Settings = Depends(get_user_settings),
conversation_store: ConversationStore = Depends(get_conversation_store),
) -> ConversationResponse:
@@ -471,7 +472,22 @@ async def start_conversation(
to start a conversation. If the conversation is already running, it will
return the existing agent loop info.
"""
logger.info(f'Starting conversation: {conversation_id}')
logger.info(
f'Starting conversation: {conversation_id}',
extra={'session_id': conversation_id},
)
# Log token fetch status
if provider_tokens:
logger.info(
f'/start endpoint: Fetched provider tokens: {list(provider_tokens.keys())}',
extra={'session_id': conversation_id},
)
else:
logger.warning(
'/start endpoint: No provider tokens fetched (provider_tokens is None/empty)',
extra={'session_id': conversation_id},
)
try:
# Check that the conversation exists
@@ -488,7 +504,7 @@ async def start_conversation(
# Set up conversation init data with provider information
conversation_init_data = await setup_init_conversation_settings(
user_id, conversation_id, providers_set.providers_set or []
user_id, conversation_id, providers_set.providers_set or [], provider_tokens
)
# Start the agent loop
+1 -23
View File
@@ -11,15 +11,10 @@ from openhands.server.routes.secrets import invalidate_legacy_secrets_store
from openhands.server.settings import (
GETSettingsModel,
)
from openhands.server.settings_validation import (
check_llm_settings_changes,
validate_llm_settings_access,
)
from openhands.server.shared import config
from openhands.server.user_auth import (
get_provider_tokens,
get_secrets_store,
get_user_id,
get_user_settings,
get_user_settings_store,
)
@@ -140,34 +135,17 @@ async def store_llm_settings(
response_model=None,
responses={
200: {'description': 'Settings stored successfully', 'model': dict},
403: {'description': 'Subscription required for pro models', 'model': dict},
500: {'description': 'Error storing settings', 'model': dict},
},
)
async def store_settings(
settings: Settings,
settings_store: SettingsStore = Depends(get_user_settings_store),
user_id: str = Depends(get_user_id),
) -> JSONResponse:
# Check provider tokens are valid
try:
existing_settings = await settings_store.load()
# Check if any LLM-related settings are being changed
llm_settings_being_changed = check_llm_settings_changes(
settings, existing_settings
)
if llm_settings_being_changed:
has_access = await validate_llm_settings_access(user_id)
if not has_access:
return JSONResponse(
status_code=status.HTTP_403_FORBIDDEN,
content={
'error': 'Modifying LLM settings requires an active OpenHands Pro subscription. Please upgrade your account to access LLM configuration.',
'detail': 'Subscription required for LLM settings modifications',
},
)
# Convert to Settings model and merge with existing settings
if existing_settings:
settings = await store_llm_settings(settings, settings_store)
@@ -215,7 +215,10 @@ def create_provider_tokens_object(
async def setup_init_conversation_settings(
user_id: str | None, conversation_id: str, providers_set: list[ProviderType]
user_id: str | None,
conversation_id: str,
providers_set: list[ProviderType],
provider_tokens: PROVIDER_TOKEN_TYPE | None = None,
) -> ConversationInitData:
"""Set up conversation initialization data with provider tokens.
@@ -223,6 +226,7 @@ async def setup_init_conversation_settings(
user_id: The user ID
conversation_id: The conversation ID
providers_set: List of provider types to set up tokens for
provider_tokens: Optional provider tokens to use (for SAAS mode resume)
Returns:
ConversationInitData with provider tokens configured
@@ -243,11 +247,30 @@ async def setup_init_conversation_settings(
session_init_args: dict = {}
session_init_args = {**settings.__dict__, **session_init_args}
git_provider_tokens = create_provider_tokens_object(providers_set)
logger.info(f'Git provider scaffold: {git_provider_tokens}')
# Use provided tokens if available (for SAAS resume), otherwise create scaffold
if provider_tokens:
logger.info(
f'Using provided provider_tokens: {list(provider_tokens.keys())}',
extra={'session_id': conversation_id},
)
git_provider_tokens = provider_tokens
else:
logger.info(
f'No provider_tokens provided, creating scaffold for: {providers_set}',
extra={'session_id': conversation_id},
)
git_provider_tokens = create_provider_tokens_object(providers_set)
logger.info(
f'Git provider scaffold: {git_provider_tokens}',
extra={'session_id': conversation_id},
)
if server_config.app_mode != AppMode.SAAS and user_secrets:
git_provider_tokens = user_secrets.provider_tokens
if server_config.app_mode != AppMode.SAAS and user_secrets:
logger.info(
f'Non-SaaS mode: Overriding with user_secrets provider tokens: {list(user_secrets.provider_tokens.keys())}',
extra={'session_id': conversation_id},
)
git_provider_tokens = user_secrets.provider_tokens
session_init_args['git_provider_tokens'] = git_provider_tokens
if user_secrets:
-122
View File
@@ -1,122 +0,0 @@
"""Settings validation utilities for LLM settings access control."""
from openhands.core.logger import openhands_logger as logger
from openhands.server.shared import server_config
from openhands.server.types import AppMode
from openhands.storage.data_models.settings import Settings
def _is_llm_setting_changing(setting_name: str, new_value, existing_settings) -> bool:
"""Check if a specific LLM setting is being changed from its existing value.
Args:
setting_name: Name of the setting to check
new_value: New value being set
existing_settings: Existing settings object (can be None)
Returns:
bool: True if the setting is being changed, False otherwise
"""
if new_value is None:
return False
# Handle special case for enable_default_condenser with default value
if setting_name == 'enable_default_condenser':
if not existing_settings:
# First time setting - only validate if setting to non-default value
return not new_value
else:
# Changing existing value
return new_value != existing_settings.enable_default_condenser
# For other settings, validate if explicitly provided and different from existing
if not existing_settings:
return True
existing_value = getattr(existing_settings, setting_name, None)
return new_value != existing_value
def check_llm_settings_changes(settings: Settings, existing_settings) -> bool:
"""Check if any LLM-related settings are being changed.
Validates both core LLM settings (model, API key, base URL) and advanced settings
shown to SaaS users (confirmation mode, security analyzer, memory condenser settings).
Args:
settings: New settings being applied
existing_settings: Current settings (can be None)
Returns:
bool: True if any LLM settings are being changed, False otherwise
"""
# Core LLM settings - always validate if provided
core_llm_changes = any(
[
settings.llm_model is not None,
settings.llm_api_key is not None,
settings.llm_base_url is not None,
]
)
if core_llm_changes:
return True
# Additional LLM settings shown to SaaS users - validate if actually changing
advanced_llm_changes = any(
[
_is_llm_setting_changing(
'confirmation_mode', settings.confirmation_mode, existing_settings
),
_is_llm_setting_changing(
'security_analyzer', settings.security_analyzer, existing_settings
),
_is_llm_setting_changing(
'enable_default_condenser',
settings.enable_default_condenser,
existing_settings,
),
_is_llm_setting_changing(
'condenser_max_size', settings.condenser_max_size, existing_settings
),
]
)
return advanced_llm_changes
async def validate_llm_settings_access(user_id: str) -> bool:
"""Validate if user has access to modify LLM settings in SaaS mode.
In SaaS mode, only pro users with active subscriptions can modify LLM settings.
Args:
user_id: The user ID to check subscription for
Returns:
bool: True if user can modify LLM settings, False otherwise
"""
# Skip validation in non-SaaS mode
if server_config.app_mode != AppMode.SAAS:
return True
# In SaaS mode, check for active subscription for ANY LLM settings changes
try:
# Import here to avoid circular imports and handle enterprise mode gracefully
from enterprise.server.routes.billing import get_subscription_access
subscription = await get_subscription_access(user_id)
# The get_subscription_access function already filters for ACTIVE status,
# so if we get a subscription back, it means it's active
return subscription is not None
except ImportError:
# Enterprise billing module not available - in SaaS mode, this means
# we can't validate subscriptions, so deny access to be safe
logger.warning(
'Enterprise billing module not available in SaaS mode, denying LLM settings access'
)
return False
except Exception as e:
# On error, deny access to be safe
logger.warning(f'Error checking subscription access for user {user_id}: {e}')
return False
@@ -297,7 +297,7 @@ async def test_github_search_repositories_with_organizations():
# First call should be for user repositories
user_call = calls[0]
user_params = user_call[0][1] # Second argument is params
assert user_params['q'] == 'openhands user:testuser'
assert user_params['q'] == 'in:name openhands user:testuser'
# Second call should be for first organization
org1_call = calls[1]
@@ -1,329 +0,0 @@
"""Security tests for settings API to ensure pro-only features are properly validated on backend."""
import os
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from fastapi import Request
from fastapi.testclient import TestClient
from pydantic import SecretStr
from openhands.integrations.provider import ProviderToken, ProviderType
from openhands.server.app import app
from openhands.server.types import AppMode
from openhands.server.user_auth.user_auth import UserAuth
from openhands.storage.data_models.user_secrets import UserSecrets
from openhands.storage.memory import InMemoryFileStore
from openhands.storage.secrets.secrets_store import SecretsStore
from openhands.storage.settings.file_settings_store import FileSettingsStore
from openhands.storage.settings.settings_store import SettingsStore
class MockUserAuthNonPro(UserAuth):
"""Mock implementation of UserAuth for non-pro user testing"""
def __init__(self):
self._settings = None
self._settings_store = MagicMock()
self._settings_store.load = AsyncMock(return_value=None)
self._settings_store.store = AsyncMock()
async def get_user_id(self) -> str | None:
return 'test-user-nonpro'
async def get_user_email(self) -> str | None:
return 'nonpro@test.com'
async def get_access_token(self) -> SecretStr | None:
return SecretStr('test-token-nonpro')
async def get_provider_tokens(self) -> dict[ProviderType, ProviderToken] | None:
return None
async def get_user_settings_store(self) -> SettingsStore | None:
return self._settings_store
async def get_secrets_store(self) -> SecretsStore | None:
return None
async def get_user_secrets(self) -> UserSecrets | None:
return None
@classmethod
async def get_instance(cls, request: Request) -> UserAuth:
return MockUserAuthNonPro()
class MockUserAuthPro(UserAuth):
"""Mock implementation of UserAuth for pro user testing"""
def __init__(self):
self._settings = None
self._settings_store = MagicMock()
self._settings_store.load = AsyncMock(return_value=None)
self._settings_store.store = AsyncMock()
async def get_user_id(self) -> str | None:
return 'test-user-pro'
async def get_user_email(self) -> str | None:
return 'pro@test.com'
async def get_access_token(self) -> SecretStr | None:
return SecretStr('test-token-pro')
async def get_provider_tokens(self) -> dict[ProviderType, ProviderToken] | None:
return None
async def get_user_settings_store(self) -> SettingsStore | None:
return self._settings_store
async def get_secrets_store(self) -> SecretsStore | None:
return None
async def get_user_secrets(self) -> UserSecrets | None:
return None
@classmethod
async def get_instance(cls, request: Request) -> UserAuth:
return MockUserAuthPro()
@pytest.fixture
def test_client_non_pro():
"""Test client for non-pro user"""
with (
patch.dict(
os.environ, {'SESSION_API_KEY': '', 'APP_MODE': 'saas'}, clear=False
),
patch('openhands.server.dependencies._SESSION_API_KEY', None),
patch('openhands.server.shared.server_config.app_mode', AppMode.SAAS),
patch(
'openhands.server.user_auth.user_auth.UserAuth.get_instance',
return_value=MockUserAuthNonPro(),
),
patch(
'openhands.storage.settings.file_settings_store.FileSettingsStore.get_instance',
AsyncMock(return_value=FileSettingsStore(InMemoryFileStore())),
),
# Mock the validation function at the routes level to return False (no access)
patch(
'openhands.server.routes.settings.validate_llm_settings_access',
AsyncMock(return_value=False),
),
):
client = TestClient(app)
yield client
@pytest.fixture
def test_client_pro():
"""Test client for pro user"""
with (
patch.dict(
os.environ, {'SESSION_API_KEY': '', 'APP_MODE': 'saas'}, clear=False
),
patch('openhands.server.dependencies._SESSION_API_KEY', None),
patch('openhands.server.shared.server_config.app_mode', AppMode.SAAS),
patch(
'openhands.server.user_auth.user_auth.UserAuth.get_instance',
return_value=MockUserAuthPro(),
),
patch(
'openhands.storage.settings.file_settings_store.FileSettingsStore.get_instance',
AsyncMock(return_value=FileSettingsStore(InMemoryFileStore())),
),
# Mock the validation function at the routes level to return True (has access)
patch(
'openhands.server.routes.settings.validate_llm_settings_access',
AsyncMock(return_value=True),
),
):
client = TestClient(app)
yield client
# Test data constants
OPENHANDS_PRO_MODELS = [
'openhands/claude-sonnet-4-20250514',
'openhands/gpt-5-2025-08-07',
'openhands/gpt-5-mini-2025-08-07',
'openhands/claude-opus-4-20250514',
'openhands/claude-opus-4-1-20250805',
'openhands/gemini-2.5-pro',
'openhands/o3',
'openhands/o4-mini',
]
DEFAULT_MODEL = 'claude-sonnet-4-20250514'
USER_PROVIDED_MODELS = [
'anthropic/claude-3-5-sonnet-20241022',
'openai/gpt-4o',
'mistral/mistral-large',
]
# Helper functions
def create_base_settings(**overrides):
"""Create base settings data with optional overrides"""
base_settings = {
'language': 'en',
'agent': 'test-agent',
'max_iterations': 100,
}
base_settings.update(overrides)
return base_settings
def assert_forbidden_response(response, model_or_setting_name=''):
"""Assert that response is 403 with subscription-related error"""
assert response.status_code == 403, (
f'{model_or_setting_name} should be forbidden for non-pro users'
)
response_data = response.json()
assert any(
keyword in response_data.get('detail', '').lower()
or keyword in response_data.get('error', '').lower()
for keyword in ['subscription', 'pro', 'upgrade']
)
@pytest.mark.parametrize(
'model',
[
'openhands/claude-sonnet-4-20250514',
'openhands/gpt-5-2025-08-07',
'openhands/claude-opus-4-20250514',
DEFAULT_MODEL,
]
+ USER_PROVIDED_MODELS,
)
@pytest.mark.asyncio
async def test_non_pro_user_cannot_set_any_llm_model(test_client_non_pro, model):
"""SECURITY TEST: Non-pro user should not be able to set any LLM model"""
settings_data = create_base_settings(llm_model=model, llm_api_key='test-key')
response = test_client_non_pro.post('/api/settings', json=settings_data)
assert_forbidden_response(response, f'Model {model}')
@pytest.mark.parametrize(
'llm_setting,value',
[
('llm_api_key', 'new-api-key'),
('llm_base_url', 'https://custom-api.example.com'),
('llm_model', DEFAULT_MODEL),
('confirmation_mode', True),
('security_analyzer', 'llm'),
('enable_default_condenser', False),
('condenser_max_size', 50),
],
)
@pytest.mark.asyncio
async def test_non_pro_user_cannot_set_individual_llm_settings(
test_client_non_pro, llm_setting, value
):
"""SECURITY TEST: Non-pro user should not be able to set individual LLM settings"""
settings_data = create_base_settings(**{llm_setting: value})
response = test_client_non_pro.post('/api/settings', json=settings_data)
assert_forbidden_response(response, f'LLM setting {llm_setting}')
@pytest.mark.asyncio
async def test_non_pro_user_can_set_non_llm_settings(test_client_non_pro):
"""Non-pro users should still be able to modify non-LLM settings"""
# Only use settings that definitely don't trigger LLM validation
settings_data = {
'language': 'fr',
'max_iterations': 50,
'user_consents_to_analytics': True,
'git_user_name': 'test-user',
'git_user_email': 'test@example.com',
}
response = test_client_non_pro.post('/api/settings', json=settings_data)
assert response.status_code == 200
@pytest.mark.asyncio
async def test_pro_user_can_set_llm_models(test_client_pro):
"""Pro user should be able to set any LLM models"""
settings_data = create_base_settings(
llm_model='openhands/claude-sonnet-4-20250514', llm_api_key='test-key'
)
response = test_client_pro.post('/api/settings', json=settings_data)
assert response.status_code == 200
@pytest.mark.asyncio
async def test_expired_subscription_cannot_access_llm_settings():
"""SECURITY TEST: User with expired subscription should not access LLM settings"""
with (
patch.dict(
os.environ, {'SESSION_API_KEY': '', 'APP_MODE': 'saas'}, clear=False
),
patch('openhands.server.dependencies._SESSION_API_KEY', None),
patch('openhands.server.shared.server_config.app_mode', AppMode.SAAS),
patch(
'openhands.server.user_auth.user_auth.UserAuth.get_instance',
return_value=MockUserAuthPro(),
),
patch(
'openhands.storage.settings.file_settings_store.FileSettingsStore.get_instance',
AsyncMock(return_value=FileSettingsStore(InMemoryFileStore())),
),
# Mock validation to return False (expired subscription, no access)
patch(
'openhands.server.routes.settings.validate_llm_settings_access',
AsyncMock(return_value=False),
),
):
client = TestClient(app)
settings_data = create_base_settings(
llm_model='openhands/claude-sonnet-4-20250514', llm_api_key='test-key'
)
response = client.post('/api/settings', json=settings_data)
assert_forbidden_response(response, 'Expired subscription')
@pytest.mark.asyncio
async def test_direct_api_bypass_prevention(test_client_non_pro):
"""SECURITY TEST: Direct API calls should still validate subscription status"""
settings_data = create_base_settings(
llm_model='openhands/claude-sonnet-4-20250514',
llm_api_key='fake-api-key',
llm_base_url='https://api.anthropic.com',
remote_runtime_resource_factor=4,
)
response = test_client_non_pro.post(
'/api/settings',
json=settings_data,
headers={
'Content-Type': 'application/json',
'User-Agent': 'DirectAPIClient/1.0',
},
)
assert_forbidden_response(response, 'Direct API bypass attempt')
@pytest.mark.parametrize(
'malicious_model',
[
'openhands/claude-sonnet-4-20250514', # Direct
'OPENHANDS/claude-sonnet-4-20250514', # Case manipulation
' openhands/claude-sonnet-4-20250514', # Leading space
'openhands//claude-sonnet-4-20250514', # Double slash
],
)
@pytest.mark.asyncio
async def test_model_prefix_bypass_attempts_blocked(
test_client_non_pro, malicious_model
):
"""SECURITY TEST: Various prefix bypass attempts should be blocked"""
settings_data = create_base_settings(
llm_model=malicious_model, llm_api_key='test-key'
)
response = test_client_non_pro.post('/api/settings', json=settings_data)
assert_forbidden_response(
response, f"Bypass attempt with model '{malicious_model}'"
)
@@ -0,0 +1,150 @@
"""Tests for /start endpoint provider token handling."""
from types import MappingProxyType
from unittest.mock import AsyncMock, patch
import pytest
from pydantic import SecretStr
from openhands.integrations.provider import ProviderToken, ProviderType
from openhands.server.data_models.agent_loop_info import AgentLoopInfo
from openhands.server.routes.manage_conversations import (
ProvidersSetModel,
start_conversation,
)
from openhands.server.types import AppMode
from openhands.storage.data_models.conversation_metadata import ConversationMetadata
from openhands.storage.data_models.conversation_status import ConversationStatus
from openhands.storage.data_models.settings import Settings
@pytest.fixture
def mock_settings():
"""Create a real Settings object with minimal required fields."""
return Settings(
language='en',
agent='CodeActAgent',
max_iterations=100,
llm_model='anthropic/claude-3-5-sonnet-20241022',
llm_api_key=SecretStr('test_api_key_12345'),
llm_base_url=None,
)
@pytest.fixture
def mock_provider_tokens():
"""Create real provider tokens to test with."""
return MappingProxyType(
{
ProviderType.GITHUB: ProviderToken(
token=SecretStr('ghp_real_token_test123'), user_id='test_user_456'
)
}
)
@pytest.fixture
def mock_conversation_metadata():
"""Create a real ConversationMetadata object."""
return ConversationMetadata(
conversation_id='test_conv_123',
user_id='test_user_456',
title='Test Conversation',
selected_repository='test/repo',
selected_branch='main',
git_provider=ProviderType.GITHUB,
)
@pytest.mark.asyncio
async def test_start_endpoint_passes_provider_tokens(
mock_settings, mock_provider_tokens, mock_conversation_metadata
):
"""Test that /start endpoint passes provider_tokens to setup_init_conversation_settings.
This test verifies the full end-to-end flow with real tokens through to ConversationInitData.
"""
conversation_id = 'test_conv_123'
user_id = 'test_user_456'
providers_set = ProvidersSetModel(providers_set=[ProviderType.GITHUB])
# Mock conversation store
mock_conversation_store = AsyncMock()
mock_conversation_store.get_metadata = AsyncMock(
return_value=mock_conversation_metadata
)
# Mock agent loop info that will be returned
mock_agent_loop_info = AgentLoopInfo(
conversation_id=conversation_id,
url=None,
session_api_key=None,
event_store=None,
status=ConversationStatus.RUNNING,
)
# Mock only infrastructure - let setup_init_conversation_settings run for real
with patch(
'openhands.server.routes.manage_conversations.conversation_manager'
) as mock_manager:
# Mock the stores that setup_init_conversation_settings needs
with patch(
'openhands.server.services.conversation_service.SettingsStoreImpl.get_instance'
) as mock_settings_store_cls:
with patch(
'openhands.server.services.conversation_service.SecretsStoreImpl.get_instance'
) as mock_secrets_store_cls:
with patch(
'openhands.server.services.conversation_service.server_config'
) as mock_server_config:
# Setup store mocks
mock_settings_store = AsyncMock()
mock_settings_store.load = AsyncMock(return_value=mock_settings)
mock_settings_store_cls.return_value = mock_settings_store
mock_secrets_store = AsyncMock()
mock_secrets_store.load = AsyncMock(return_value=None)
mock_secrets_store_cls.return_value = mock_secrets_store
mock_server_config.app_mode = AppMode.SAAS
mock_manager.maybe_start_agent_loop = AsyncMock(
return_value=mock_agent_loop_info
)
# Call endpoint with provider tokens
response = await start_conversation(
providers_set=providers_set,
conversation_id=conversation_id,
user_id=user_id,
provider_tokens=mock_provider_tokens,
settings=mock_settings,
conversation_store=mock_conversation_store,
)
# Verify ConversationInitData has real provider tokens
mock_manager.maybe_start_agent_loop.assert_called_once()
call_kwargs = mock_manager.maybe_start_agent_loop.call_args[1]
conversation_init_data = call_kwargs['settings']
assert conversation_init_data.git_provider_tokens is not None
assert (
conversation_init_data.git_provider_tokens
== mock_provider_tokens
)
assert (
ProviderType.GITHUB
in conversation_init_data.git_provider_tokens
)
github_token = conversation_init_data.git_provider_tokens[
ProviderType.GITHUB
]
assert (
github_token.token.get_secret_value()
== 'ghp_real_token_test123'
)
assert github_token.user_id == 'test_user_456'
assert response.status == 'ok'
assert response.conversation_id == conversation_id
@@ -0,0 +1,162 @@
"""Unit tests for conversation_service.py - specifically testing provider token handling.
These tests verify that setup_init_conversation_settings correctly handles provider tokens
in different scenarios (provided tokens vs scaffold creation).
"""
from types import MappingProxyType
from unittest.mock import AsyncMock, MagicMock, patch
import pytest
from pydantic import SecretStr
from openhands.integrations.provider import ProviderToken, ProviderType
from openhands.server.services.conversation_service import (
setup_init_conversation_settings,
)
from openhands.server.types import AppMode
from openhands.storage.data_models.settings import Settings
@pytest.fixture
def mock_settings():
"""Create a real Settings object with minimal required fields."""
return Settings(
language='en',
agent='CodeActAgent',
max_iterations=100,
llm_model='anthropic/claude-3-5-sonnet-20241022',
llm_api_key=SecretStr('test_api_key_12345'),
llm_base_url=None,
)
@pytest.fixture
def mock_provider_tokens():
"""Create real provider tokens to test with."""
return MappingProxyType(
{
ProviderType.GITHUB: ProviderToken(
token=SecretStr('ghp_real_token_test123'), user_id='test_user_456'
)
}
)
@pytest.mark.asyncio
async def test_setup_with_provided_tokens_uses_real_tokens(
mock_settings, mock_provider_tokens
):
"""Test that real tokens are used when provided in SAAS mode.
Verifies provider tokens passed in are used in ConversationInitData.
"""
user_id = 'test_user_456'
conversation_id = 'test_conv_123'
providers_set = [ProviderType.GITHUB]
# Mock the stores to return our test settings
with patch(
'openhands.server.services.conversation_service.SettingsStoreImpl.get_instance'
) as mock_settings_store_cls:
with patch(
'openhands.server.services.conversation_service.SecretsStoreImpl.get_instance'
) as mock_secrets_store_cls:
with patch(
'openhands.server.services.conversation_service.server_config'
) as mock_server_config:
# Setup mocks
mock_settings_store = AsyncMock()
mock_settings_store.load = AsyncMock(return_value=mock_settings)
mock_settings_store_cls.return_value = mock_settings_store
mock_secrets_store = AsyncMock()
mock_secrets_store.load = AsyncMock(return_value=None)
mock_secrets_store_cls.return_value = mock_secrets_store
mock_server_config.app_mode = AppMode.SAAS
# Call with real tokens
result = await setup_init_conversation_settings(
user_id=user_id,
conversation_id=conversation_id,
providers_set=providers_set,
provider_tokens=mock_provider_tokens,
)
# Verify real tokens are used
assert result.git_provider_tokens is not None
assert result.git_provider_tokens == mock_provider_tokens
assert ProviderType.GITHUB in result.git_provider_tokens, (
'GitHub provider should be in tokens'
)
github_token = result.git_provider_tokens[ProviderType.GITHUB]
assert (
github_token.token.get_secret_value() == 'ghp_real_token_test123'
), 'Should use real token, not None'
assert github_token.user_id == 'test_user_456', (
'Should preserve user_id from real token'
)
@pytest.mark.asyncio
async def test_setup_without_tokens_non_saas_uses_user_secrets(mock_settings):
"""Test that OSS mode uses user_secrets.provider_tokens when no tokens provided.
This test verifies OSS mode backward compatibility - tokens come from local config, not endpoint.
"""
user_id = 'test_user_456'
conversation_id = 'test_conv_123'
providers_set = [ProviderType.GITHUB]
# Create user_secrets with real tokens
mock_user_secrets = MagicMock()
mock_user_secrets.provider_tokens = MappingProxyType(
{
ProviderType.GITHUB: ProviderToken(
token=SecretStr('ghp_local_token_from_config'),
user_id='local_user_123',
)
}
)
mock_user_secrets.custom_secrets = MappingProxyType({}) # Empty dict is fine
with patch(
'openhands.server.services.conversation_service.SettingsStoreImpl.get_instance'
) as mock_settings_store_cls:
with patch(
'openhands.server.services.conversation_service.SecretsStoreImpl.get_instance'
) as mock_secrets_store_cls:
with patch(
'openhands.server.services.conversation_service.server_config'
) as mock_server_config:
# Setup mocks
mock_settings_store = AsyncMock()
mock_settings_store.load = AsyncMock(return_value=mock_settings)
mock_settings_store_cls.return_value = mock_settings_store
mock_secrets_store = AsyncMock()
mock_secrets_store.load = AsyncMock(return_value=mock_user_secrets)
mock_secrets_store_cls.return_value = mock_secrets_store
mock_server_config.app_mode = AppMode.OSS
# Call without endpoint tokens
result = await setup_init_conversation_settings(
user_id=user_id,
conversation_id=conversation_id,
providers_set=providers_set,
provider_tokens=None,
)
# Verify user_secrets tokens are used
assert result.git_provider_tokens is not None
assert ProviderType.GITHUB in result.git_provider_tokens
github_token = result.git_provider_tokens[ProviderType.GITHUB]
assert (
github_token.token.get_secret_value()
== 'ghp_local_token_from_config'
)
assert github_token.user_id == 'local_user_123'