feat(core): enforce tenant isolation guard outside production
Non-production environments (development/testing) now force the effective tenant to the reserved test tenant "llm_tester" (or a test_-prefixed override) regardless of DEFAULT_USER misconfiguration: - Config.effective_default_user only honors DEFAULT_USER outside production when it is llm_tester or test_-prefixed; anything else is forced to llm_tester (tenant_forced flags the override) - Config refuses startup (validation error) when a non-production environment is explicitly configured with the production tenant jpmschweitzer - get_user() applies the same guard at request-context resolution, so an explicit request for the production tenant in dev/test is forced to llm_tester with a warning log - initialize_application() emits one loud startup log line (tenant_guard_active / tenant_guard_production) stating the effective tenant Unit tests cover the dev/test/prod x default/explicit-user matrix. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -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)
|
||||
|
||||
+55
-8
@@ -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
|
||||
|
||||
+30
-1
@@ -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:
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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},
|
||||
)
|
||||
]
|
||||
Reference in New Issue
Block a user