diff --git a/CHANGELOG.md b/CHANGELOG.md index 72c75fd..7e412cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added +- **Tenant isolation guard** - non-production environments (development/testing) now FORCE the effective tenant to the reserved test tenant `llm_tester` (only `llm_tester` itself or a `test_`-prefixed override is accepted), regardless of `DEFAULT_USER` misconfiguration, at both config resolution and request-context resolution (`get_user()`). Startup refuses (clear error) when a non-production environment is explicitly configured with the production tenant `jpmschweitzer`, and one loud startup log line states the effective/forced tenant + - **Conversation context for experts + real-time think messages** - direct delegation (streaming and non-streaming) now passes a trimmed conversation history (last 6 turns) as expert context, so follow-up questions keep their referent; `_stream_direct_delegation` is now an async generator, so butler think messages ("Allow me to consult the archives, sir.") stream BEFORE the research runs instead of after it completes - **Bounded retries and connection reuse for library-desk** - GETs and the read-only `POST /query/*` and `POST /rag/search` endpoints retry once (2 attempts, short backoff) on transport errors and retryable 5xx; wiki writes are never retried. The client now honors `LIBRARY_DESK_TIMEOUT` instead of hardcoded 60s/30s, a librarian run holds one shared HTTP connection instead of constructing a client per tool call, and read tools raise `ModelRetry` on transient HTTP errors so the agent's retry budget engages - **One librarian timeout budget** - new `LIBRARIAN_TIMEOUT` (default 180s) enforced with `asyncio.wait_for` inside `delegate_to_librarian`, capping the previously uncapped live paths (steward direct delegation and streaming). The Ollama provider's AsyncOpenAI client now carries an explicit `OLLAMA_TIMEOUT` instead of the SDK's ~600s default, and the contradictory unused 60s default in `AgentRequest.timeout_seconds` was removed (None defers to the configured budget) diff --git a/src/core/config.py b/src/core/config.py index 2d3448d..05d04eb 100644 --- a/src/core/config.py +++ b/src/core/config.py @@ -6,9 +6,17 @@ from enum import Enum from functools import lru_cache from pathlib import Path -from pydantic import Field, HttpUrl +from pydantic import Field, HttpUrl, model_validator from pydantic_settings import BaseSettings, SettingsConfigDict +# Tenant isolation constants (see docs: tenant-based isolation, no separate +# test infrastructure). The production tenant owns real data in the shared +# services (Qdrant/Neo4j/Wiki.js/Redis); everything non-production must run +# under the reserved test tenant or an explicit test_-prefixed namespace. +PRODUCTION_TENANT = "jpmschweitzer" +TEST_TENANT = "llm_tester" +TEST_TENANT_PREFIX = "test_" + def _get_version_from_pyproject() -> str: """ @@ -207,6 +215,29 @@ class Config(BaseSettings): CORS_ALLOW_METHODS: list[str] = ["*"] CORS_ALLOW_HEADERS: list[str] = ["*"] + @model_validator(mode="after") + def _refuse_production_tenant_outside_production(self) -> "Config": + """ + Refuse startup when a non-production environment is explicitly + configured with the production tenant. + + This is the hard stop of the tenant isolation guard: a dev/test + instance must never be able to read or write the production + tenant's data in the shared services. + """ + if ( + self.ENVIRONMENT != Environment.PRODUCTION + and self.DEFAULT_USER == PRODUCTION_TENANT + ): + raise ValueError( + f"Refusing to start: ENVIRONMENT={self.ENVIRONMENT.value} is " + f"explicitly configured with the production tenant " + f"'{PRODUCTION_TENANT}'. Non-production environments must use " + f"'{TEST_TENANT}' or a '{TEST_TENANT_PREFIX}'-prefixed tenant. " + f"Unset DEFAULT_USER or set ENVIRONMENT=production." + ) + return self + @property def redis_memory_url(self) -> str: """Construct Redis connection URL for memory cache.""" @@ -247,16 +278,32 @@ class Config(BaseSettings): @property def effective_default_user(self) -> str: """ - Get effective default user, auto-determining from environment if not set. + Get effective default user (tenant), enforcing tenant isolation. - - development/testing: llm_tester (isolated test scope) - - production: jpmschweitzer (real user) + - production: DEFAULT_USER if set, else the production tenant + - development/testing: FORCED to the reserved test tenant + ("llm_tester") - the only accepted overrides are the test tenant + itself or a "test_"-prefixed namespace. Any other DEFAULT_USER + value is treated as misconfiguration and ignored. """ - if self.DEFAULT_USER is not None: - return self.DEFAULT_USER if self.ENVIRONMENT == Environment.PRODUCTION: - return "jpmschweitzer" - return "llm_tester" + return self.DEFAULT_USER or PRODUCTION_TENANT + + if self.DEFAULT_USER is not None and ( + self.DEFAULT_USER == TEST_TENANT + or self.DEFAULT_USER.startswith(TEST_TENANT_PREFIX) + ): + return self.DEFAULT_USER + return TEST_TENANT + + @property + def tenant_forced(self) -> bool: + """Whether the tenant guard overrode a misconfigured DEFAULT_USER.""" + return ( + self.ENVIRONMENT != Environment.PRODUCTION + and self.DEFAULT_USER is not None + and self.effective_default_user != self.DEFAULT_USER + ) @lru_cache diff --git a/src/core/context.py b/src/core/context.py index 8533c93..9abb159 100644 --- a/src/core/context.py +++ b/src/core/context.py @@ -41,6 +41,33 @@ current_conversation: ContextVar[str | None] = ContextVar( ) +def _apply_tenant_guard(user: str) -> str: + """ + Enforce tenant isolation at request-context resolution. + + In non-production environments the production tenant must never be + the effective user - a request that explicitly asks for it is forced + to the reserved test tenant instead (with a loud log line). + """ + # Import here to avoid circular dependency + from src.core.config import PRODUCTION_TENANT, TEST_TENANT, Environment, config + + if ( + config.ENVIRONMENT != Environment.PRODUCTION + and user == PRODUCTION_TENANT + ): + from src.core.logging_config import get_logger + + get_logger(__name__).warning( + "tenant_guard_forced", + environment=config.ENVIRONMENT.value, + requested_tenant=user, + forced_tenant=TEST_TENANT, + ) + return TEST_TENANT + return user + + def get_user() -> str: """ Get current user from request context. @@ -48,6 +75,8 @@ def get_user() -> str: Returns: User identifier for the current request. Falls back to environment-aware default if not set. + In non-production environments the production tenant is never + returned - the tenant guard forces the reserved test tenant. Example: user = get_user() # "llm_tester" (dev) or "jpmschweitzer" (prod) @@ -55,7 +84,7 @@ def get_user() -> str: user = current_user.get() if user == _USER_NOT_SET: return get_default_user() - return user + return _apply_tenant_guard(user) def get_conversation_id() -> str | None: diff --git a/src/core/startup.py b/src/core/startup.py index 53e26f5..b7fa8f9 100644 --- a/src/core/startup.py +++ b/src/core/startup.py @@ -14,12 +14,38 @@ from src.anthropic.model_selector import ( check_ollama_health, get_model_info, ) +from src.core.config import Environment, config from src.core.household_registry import get_household_registry from src.core.logging_config import get_logger logger = get_logger(__name__) +def log_tenant_guard() -> None: + """ + Emit one loud startup log line stating the effective tenant. + + In non-production environments the tenant guard forces the reserved + test tenant regardless of DEFAULT_USER misconfiguration - this line + makes that override visible at startup. + """ + if config.ENVIRONMENT == Environment.PRODUCTION: + logger.info( + "tenant_guard_production", + environment=config.ENVIRONMENT.value, + tenant=config.effective_default_user, + ) + return + + logger.warning( + "tenant_guard_active", + environment=config.ENVIRONMENT.value, + forced_tenant=config.effective_default_user, + default_user_overridden=config.tenant_forced, + configured_default_user=config.DEFAULT_USER, + ) + + def register_household_members(): """ Register all household members with the registry. @@ -99,6 +125,9 @@ async def initialize_application(): """ logger.info("application_initialization_starting") + # Tenant isolation guard: state the effective tenant loudly + log_tenant_guard() + # Check backend health: Ollama is primary, Claude is the fallback await check_ollama_health() await check_claude_health() diff --git a/tests/core/test_tenant_guard.py b/tests/core/test_tenant_guard.py new file mode 100644 index 0000000..4e020fd --- /dev/null +++ b/tests/core/test_tenant_guard.py @@ -0,0 +1,200 @@ +""" +Tests for the tenant isolation guard. + +Isolation is tenant-based: the production tenant ("jpmschweitzer") owns +real data in the shared services, and every non-production environment +must run under the reserved test tenant ("llm_tester") or an explicit +"test_"-prefixed namespace. + +Guard matrix covered here: dev/test/prod x default/explicit user, at +both config level (effective_default_user) and request-context +resolution (get_user). +""" + +import pytest +from pydantic import ValidationError + +from src.core.config import ( + PRODUCTION_TENANT, + TEST_TENANT, + Config, + Environment, +) +from src.core.context import RequestContext, get_user + + +def make_config(**overrides) -> Config: + """Build a Config isolated from the local .env file.""" + return Config(_env_file=None, **overrides) + + +@pytest.mark.unit +class TestEffectiveDefaultUserMatrix: + """Config-level guard: effective_default_user per environment.""" + + # --- development --- + + def test_dev_without_default_user_forces_test_tenant(self): + config = make_config(ENVIRONMENT=Environment.DEVELOPMENT) + assert config.effective_default_user == TEST_TENANT + assert config.tenant_forced is False + + def test_dev_with_test_tenant_is_kept(self): + config = make_config(ENVIRONMENT=Environment.DEVELOPMENT, DEFAULT_USER=TEST_TENANT) + assert config.effective_default_user == TEST_TENANT + assert config.tenant_forced is False + + def test_dev_with_test_prefixed_override_is_kept(self): + config = make_config(ENVIRONMENT=Environment.DEVELOPMENT, DEFAULT_USER="test_phase_b") + assert config.effective_default_user == "test_phase_b" + assert config.tenant_forced is False + + def test_dev_with_misconfigured_user_is_forced_to_test_tenant(self): + config = make_config(ENVIRONMENT=Environment.DEVELOPMENT, DEFAULT_USER="alice") + assert config.effective_default_user == TEST_TENANT + assert config.tenant_forced is True + + def test_dev_with_production_tenant_refuses_startup(self): + with pytest.raises(ValidationError) as exc_info: + make_config( + ENVIRONMENT=Environment.DEVELOPMENT, + DEFAULT_USER=PRODUCTION_TENANT, + ) + assert "Refusing to start" in str(exc_info.value) + assert PRODUCTION_TENANT in str(exc_info.value) + + # --- testing --- + + def test_testing_without_default_user_forces_test_tenant(self): + config = make_config(ENVIRONMENT=Environment.TESTING) + assert config.effective_default_user == TEST_TENANT + + def test_testing_with_misconfigured_user_is_forced_to_test_tenant(self): + config = make_config(ENVIRONMENT=Environment.TESTING, DEFAULT_USER="bob") + assert config.effective_default_user == TEST_TENANT + assert config.tenant_forced is True + + def test_testing_with_production_tenant_refuses_startup(self): + with pytest.raises(ValidationError, match="Refusing to start"): + make_config( + ENVIRONMENT=Environment.TESTING, + DEFAULT_USER=PRODUCTION_TENANT, + ) + + def test_testing_with_test_prefixed_override_is_kept(self): + config = make_config(ENVIRONMENT=Environment.TESTING, DEFAULT_USER="test_ci_run") + assert config.effective_default_user == "test_ci_run" + + # --- production --- + + def test_prod_without_default_user_uses_production_tenant(self): + config = make_config(ENVIRONMENT=Environment.PRODUCTION) + assert config.effective_default_user == PRODUCTION_TENANT + assert config.tenant_forced is False + + def test_prod_with_explicit_production_tenant_is_kept(self): + config = make_config(ENVIRONMENT=Environment.PRODUCTION, DEFAULT_USER=PRODUCTION_TENANT) + assert config.effective_default_user == PRODUCTION_TENANT + + def test_prod_with_explicit_other_user_is_kept(self): + config = make_config(ENVIRONMENT=Environment.PRODUCTION, DEFAULT_USER="household_guest") + assert config.effective_default_user == "household_guest" + assert config.tenant_forced is False + + +@pytest.mark.unit +class TestRequestContextGuard: + """Request-context resolution guard: get_user() per environment.""" + + def _patch_environment(self, monkeypatch, environment: Environment): + from src.core import config as config_module + + monkeypatch.setattr(config_module.config, "ENVIRONMENT", environment) + + def test_dev_default_resolution_is_test_tenant(self, monkeypatch): + self._patch_environment(monkeypatch, Environment.DEVELOPMENT) + assert get_user() == TEST_TENANT + + def test_dev_explicit_production_tenant_is_forced(self, monkeypatch): + self._patch_environment(monkeypatch, Environment.DEVELOPMENT) + with RequestContext(user=PRODUCTION_TENANT): + assert get_user() == TEST_TENANT + + def test_testing_explicit_production_tenant_is_forced(self, monkeypatch): + self._patch_environment(monkeypatch, Environment.TESTING) + with RequestContext(user=PRODUCTION_TENANT): + assert get_user() == TEST_TENANT + + def test_dev_explicit_other_user_passes_through(self, monkeypatch): + self._patch_environment(monkeypatch, Environment.DEVELOPMENT) + with RequestContext(user="testuser"): + assert get_user() == "testuser" + + def test_prod_explicit_production_tenant_passes_through(self, monkeypatch): + self._patch_environment(monkeypatch, Environment.PRODUCTION) + with RequestContext(user=PRODUCTION_TENANT): + assert get_user() == PRODUCTION_TENANT + + def test_prod_explicit_other_user_passes_through(self, monkeypatch): + self._patch_environment(monkeypatch, Environment.PRODUCTION) + with RequestContext(user="alice"): + assert get_user() == "alice" + + +@pytest.mark.unit +class TestStartupTenantGuardLog: + """One loud startup log line states the effective tenant.""" + + def test_non_production_logs_forced_tenant(self, monkeypatch): + from src.core import startup as startup_module + + events = [] + + class _Recorder: + def warning(self, event, **kw): + events.append((event, kw)) + + def info(self, event, **kw): + events.append((event, kw)) + + monkeypatch.setattr(startup_module, "logger", _Recorder()) + monkeypatch.setattr(startup_module.config, "ENVIRONMENT", Environment.DEVELOPMENT) + + startup_module.log_tenant_guard() + + assert events == [ + ( + "tenant_guard_active", + { + "environment": "development", + "forced_tenant": TEST_TENANT, + "default_user_overridden": startup_module.config.tenant_forced, + "configured_default_user": startup_module.config.DEFAULT_USER, + }, + ) + ] + + def test_production_logs_production_tenant(self, monkeypatch): + from src.core import startup as startup_module + + events = [] + + class _Recorder: + def warning(self, event, **kw): + events.append(("warning", event, kw)) + + def info(self, event, **kw): + events.append(("info", event, kw)) + + monkeypatch.setattr(startup_module, "logger", _Recorder()) + monkeypatch.setattr(startup_module.config, "ENVIRONMENT", Environment.PRODUCTION) + + startup_module.log_tenant_guard() + + assert events == [ + ( + "info", + "tenant_guard_production", + {"environment": "production", "tenant": PRODUCTION_TENANT}, + ) + ]