fix(librarian): apply tenant guard to explicit user args in client
LibraryDeskClient._resolve_user only enforced non-empty: an explicit user argument to any tenant-scoped method bypassed tatlock's tenant guard entirely and went straight to library-desk, and padded values were sent un-stripped on the wire. Route the explicit-arg path through the same apply_tenant_guard() used by context resolution and strip whitespace before the empty check, so a non-production environment can never send the production tenant (or a sanitization-collision variant) to library-desk, regardless of how the user was supplied. Defense in depth - no in-repo caller passes an explicit user today. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
@@ -17,7 +17,7 @@ import httpx
|
||||
from pydantic import BaseModel, Field
|
||||
|
||||
from src.core.config import config
|
||||
from src.core.context import get_user
|
||||
from src.core.context import apply_tenant_guard, get_user
|
||||
from src.core.logging_config import get_logger
|
||||
|
||||
logger = get_logger(__name__)
|
||||
@@ -271,14 +271,19 @@ class LibraryDeskClient:
|
||||
request must carry an explicit tenant (a missing user will 422).
|
||||
An empty tenant is a programming or configuration error - fail
|
||||
loudly here, before any bytes hit the wire.
|
||||
|
||||
Explicit user arguments are stripped and routed through the same
|
||||
tenant guard as context resolution (get_user() already applies
|
||||
it), so a dev environment can never send the production tenant -
|
||||
or a sanitization-collision variant of it - to library-desk.
|
||||
"""
|
||||
effective = user if user is not None else get_user()
|
||||
if not effective or not effective.strip():
|
||||
effective = (user if user is not None else get_user()).strip()
|
||||
if not effective:
|
||||
raise ValueError(
|
||||
"library-desk request requires a non-empty user (tenant); "
|
||||
"got an empty value from the caller or request context"
|
||||
)
|
||||
return effective
|
||||
return apply_tenant_guard(effective)
|
||||
|
||||
async def _request_with_retry(
|
||||
self,
|
||||
|
||||
@@ -671,6 +671,59 @@ class TestExplicitUserContract:
|
||||
|
||||
mock_httpx.post.assert_not_called()
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_explicit_padded_user_is_stripped_on_the_wire(self):
|
||||
"""Padded explicit users are stripped, not sent verbatim."""
|
||||
client, mock_httpx = self._wire_client()
|
||||
|
||||
await client.hybrid_search("q", user=" llm_tester ")
|
||||
|
||||
assert self._sent_user(mock_httpx) == "llm_tester"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
@pytest.mark.parametrize(
|
||||
"explicit_user",
|
||||
["jpmschweitzer", "JPMSchweitzer", "jpmschweitzer.", " jpmschweitzer"],
|
||||
)
|
||||
@pytest.mark.parametrize("method_name,kwargs", TENANT_SCOPED_METHODS)
|
||||
async def test_explicit_production_tenant_is_guarded_in_dev(
|
||||
self, monkeypatch, method_name, kwargs, explicit_user
|
||||
):
|
||||
"""
|
||||
An explicit production-tenant argument (or a sanitization-collision
|
||||
variant) never reaches library-desk from a non-production
|
||||
environment - the client applies the same tenant guard as
|
||||
context resolution.
|
||||
"""
|
||||
from src.core import config as config_module
|
||||
from src.core.config import Environment
|
||||
|
||||
monkeypatch.setattr(
|
||||
config_module.config, "ENVIRONMENT", Environment.DEVELOPMENT
|
||||
)
|
||||
client, mock_httpx = self._wire_client()
|
||||
|
||||
await getattr(client, method_name)(user=explicit_user, **kwargs)
|
||||
|
||||
assert self._sent_user(mock_httpx) == "llm_tester"
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_explicit_production_tenant_passes_through_in_prod(
|
||||
self, monkeypatch
|
||||
):
|
||||
"""In production the production tenant is sent unchanged."""
|
||||
from src.core import config as config_module
|
||||
from src.core.config import Environment
|
||||
|
||||
monkeypatch.setattr(
|
||||
config_module.config, "ENVIRONMENT", Environment.PRODUCTION
|
||||
)
|
||||
client, mock_httpx = self._wire_client()
|
||||
|
||||
await client.hybrid_search("q", user="jpmschweitzer")
|
||||
|
||||
assert self._sent_user(mock_httpx) == "jpmschweitzer"
|
||||
|
||||
|
||||
@pytest.mark.unit
|
||||
class TestNewResponseModels:
|
||||
|
||||
Reference in New Issue
Block a user