ref #10: [fix] enforce UUID chat ids

Normalize chat ids to a single UUID form so locks, repository keys, and mount paths cannot diverge through path-like aliases.
This commit is contained in:
Azamat 2026-04-02 22:35:50 +03:00
parent 44f1549d80
commit e629e34c4d
7 changed files with 192 additions and 80 deletions

View file

@ -1,5 +1,6 @@
from datetime import datetime from datetime import datetime
from pathlib import Path from pathlib import Path
from uuid import UUID
from docker import DockerClient from docker import DockerClient
from docker.errors import DockerException, NotFound from docker.errors import DockerException, NotFound
@ -28,8 +29,11 @@ class DockerSandboxRuntime(SandboxRuntime):
created_at: datetime, created_at: datetime,
expires_at: datetime, expires_at: datetime,
) -> SandboxSession: ) -> SandboxSession:
normalized_chat_id = chat_id
try: try:
chat_path = self._chat_path(chat_id) normalized_chat_id = _canonical_chat_id(chat_id)
chat_path = self._chat_path(normalized_chat_id)
dependencies_path = self._readonly_host_path( dependencies_path = self._readonly_host_path(
self._config.dependencies_host_path self._config.dependencies_host_path
) )
@ -40,19 +44,19 @@ class DockerSandboxRuntime(SandboxRuntime):
container = self._client.containers.run( container = self._client.containers.run(
self._config.image, self._config.image,
detach=True, detach=True,
labels=self._labels(session_id, chat_id, expires_at), labels=self._labels(session_id, normalized_chat_id, expires_at),
mounts=self._mounts(chat_path, dependencies_path, lambda_tools_path), mounts=self._mounts(chat_path, dependencies_path, lambda_tools_path),
) )
except (DockerException, OSError, ValueError) as exc: except (DockerException, OSError, ValueError) as exc:
raise SandboxStartError(chat_id) from exc raise SandboxStartError(normalized_chat_id) from exc
container_id = str(getattr(container, 'id', '')).strip() container_id = str(getattr(container, 'id', '')).strip()
if not container_id: if not container_id:
raise SandboxStartError(chat_id) raise SandboxStartError(normalized_chat_id)
return SandboxSession( return SandboxSession(
session_id=session_id, session_id=session_id,
chat_id=chat_id, chat_id=normalized_chat_id,
container_id=container_id, container_id=container_id,
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=created_at, created_at=created_at,
@ -124,3 +128,7 @@ class DockerSandboxRuntime(SandboxRuntime):
def _host_path(self, path_value: str) -> Path: def _host_path(self, path_value: str) -> Path:
return Path(path_value).expanduser().resolve(strict=False) return Path(path_value).expanduser().resolve(strict=False)
def _canonical_chat_id(chat_id: str) -> str:
return str(UUID(str(chat_id).strip()))

View file

@ -1,6 +1,7 @@
from datetime import datetime from datetime import datetime
from uuid import UUID
from pydantic import BaseModel, ConfigDict, Field from pydantic import BaseModel, ConfigDict, Field, field_validator
class HealthResponse(BaseModel): class HealthResponse(BaseModel):
@ -14,6 +15,11 @@ class CreateSandboxRequest(BaseModel):
chat_id: str = Field(min_length=1) chat_id: str = Field(min_length=1)
@field_validator('chat_id')
@classmethod
def validate_chat_id(cls, value: str) -> str:
return str(UUID(value))
class SandboxSessionResponse(BaseModel): class SandboxSessionResponse(BaseModel):
session_id: str session_id: str

View file

@ -163,7 +163,7 @@
### M13. Повторный boundary review после fix-pass ### M13. Повторный boundary review после fix-pass
- Субагент: `code-reviewer` - Субагент: `code-reviewer`
- Статус: pending - Статус: completed
- Зависимости: `M12` - Зависимости: `M12`
- Commit required: no - Commit required: no
- Scope: проверить, что must-fix и should-fix замечания из `M08` закрыты без нарушения clean architecture - Scope: проверить, что must-fix и should-fix замечания из `M08` закрыты без нарушения clean architecture
@ -181,3 +181,38 @@
- Ошибки: несовместимый fake Docker client для `DockerSandboxRuntime`, неточная типизация `run_calls` и ASGI message payload, использование `object` вместо типизированных test doubles для `AppRepositories`, `AppUsecases`, `AppContainer` - Ошибки: несовместимый fake Docker client для `DockerSandboxRuntime`, неточная типизация `run_calls` и ASGI message payload, использование `object` вместо типизированных test doubles для `AppRepositories`, `AppUsecases`, `AppContainer`
- Решение: сделать test doubles типизированными через совместимые fake classes или локальные protocols; убрать `object` и неиндексируемые `dict[str, object]` там, где mypy не может вывести типы - Решение: сделать test doubles типизированными через совместимые fake classes или локальные protocols; убрать `object` и неиндексируемые `dict[str, object]` там, где mypy не может вывести типы
- Критерии приемки: `uv run mypy .` проходит; `make pre-commit` доходит как минимум до pytest stage; production code не меняется или меняется только при явной необходимости для testability - Критерии приемки: `uv run mypy .` проходит; `make pre-commit` доходит как минимум до pytest stage; production code не меняется или меняется только при явной необходимости для testability
## Follow-up после M13 review
### M15. Канонизация и валидация `chat_id`
- Субагент: `feature-developer`
- Статус: completed
- Зависимости: `M13`
- Commit required: no
- Scope: сделать `chat_id` строго UUID и убрать path alias/whole-root mount риск через неканоничные значения
- Файлы: `adapter/http/fastapi/schemas.py`, `adapter/docker/runtime.py`, при необходимости `usecase/sandbox.py` и тесты в `test/*`
- Решение: принять `chat_id` как UUID на HTTP boundary, использовать его каноничную строковую форму дальше в usecase/repository/path construction и не принимать произвольные path-like строки
- Критерии приемки: не-UUID значения отклоняются на HTTP boundary с `400/422`; UUID используется как единое каноничное значение для lock key, repository key и filesystem path; появляются регрессионные тесты на invalid `chat_id`
### M16. Lifecycle reconciliation на startup/shutdown
- Субагент: `feature-developer`
- Статус: pending
- Зависимости: `M13`
- Commit required: no
- Scope: устранить restart-gap между in-memory registry и уже запущенными Docker containers
- Файлы: `adapter/docker/runtime.py`, `adapter/di/container.py`, `adapter/http/fastapi/app.py`, при необходимости новые outer-layer helper files и тесты в `test/*`
- Решение: основная стратегия — reconciliation по Docker labels на startup, чтобы после restart master-service продолжал видеть уже запущенные sandbox и не поднимал дубликаты; graceful shutdown cleanup остается опциональным дополнением
- Критерии приемки: после restart master-service может восстановить/синхронизировать state по Docker labels без потери работающих agent containers; one-sandbox-per-chat не нарушается из-за пустого in-memory registry; lifecycle policy явно зафиксирована и покрыта тестами
### M17. Управление жизненным циклом per-chat locks
- Субагент: `feature-developer`
- Статус: pending
- Зависимости: `M13`
- Commit required: no
- Scope: ограничить неограниченный рост registry locks по числу когда-либо увиденных `chat_id`
- Файлы: `repository/sandbox_lock.py`, при необходимости тесты в `test/*`
- Решение: добавить eviction/ref-count/weakref policy во внешнем lock registry без нарушения сериализации lifecycle для активного `chat_id`
- Критерии приемки: registry locks не растет бесконечно без причины; сериализация для активных чатов сохраняется; поведение покрыто тестами

View file

@ -28,6 +28,9 @@ from repository.sandbox_session import InMemorySandboxSessionRepository
from usecase.interface import Attrs from usecase.interface import Attrs
from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand
CHAT_ID = '123e4567-e89b-12d3-a456-426614174000'
NON_CANONICAL_CHAT_ID = '123E4567E89B12D3A456426614174000'
class FakeLogger: class FakeLogger:
def __init__(self) -> None: def __init__(self) -> None:
@ -246,12 +249,12 @@ async def exercise_get_request(
await app.router.shutdown() await app.router.shutdown()
def test_post_create_returns_session(monkeypatch) -> None: def test_post_create_returns_session_with_canonical_chat_id(monkeypatch) -> None:
config = build_config() config = build_config()
expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC) expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC)
session = SandboxSession( session = SandboxSession(
session_id='session-123', session_id='session-123',
chat_id='chat-123', chat_id=CHAT_ID,
container_id='container-123', container_id='container-123',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=expires_at - timedelta(minutes=5), created_at=expires_at - timedelta(minutes=5),
@ -276,19 +279,19 @@ def test_post_create_returns_session(monkeypatch) -> None:
app = app_module.create_app(config=config) app = app_module.create_app(config=config)
status_code, response = asyncio.run( status_code, response = asyncio.run(
exercise_create_request(app, {'chat_id': 'chat-123'}) exercise_create_request(app, {'chat_id': NON_CANONICAL_CHAT_ID})
) )
assert status_code == 200 assert status_code == 200
assert response == { assert response == {
'session_id': 'session-123', 'session_id': 'session-123',
'chat_id': 'chat-123', 'chat_id': CHAT_ID,
'container_id': 'container-123', 'container_id': 'container-123',
'status': 'running', 'status': 'running',
'expires_at': '2026-04-02T12:05:00Z', 'expires_at': '2026-04-02T12:05:00Z',
} }
assert len(create_usecase.commands) == 1 assert len(create_usecase.commands) == 1
assert create_usecase.commands[0].chat_id == 'chat-123' assert create_usecase.commands[0].chat_id == CHAT_ID
assert cleanup_usecase.calls >= 1 assert cleanup_usecase.calls >= 1
assert any( assert any(
message == 'http_request' message == 'http_request'
@ -299,10 +302,19 @@ def test_post_create_returns_session(monkeypatch) -> None:
assert docker_client.close_calls == 1 assert docker_client.close_calls == 1
def test_post_create_maps_start_errors_to_service_unavailable(monkeypatch) -> None: def test_post_create_rejects_non_uuid_chat_id(monkeypatch) -> None:
config = build_config() config = build_config()
expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC)
session = SandboxSession(
session_id='session-123',
chat_id=CHAT_ID,
container_id='container-123',
status=SandboxStatus.RUNNING,
created_at=expires_at - timedelta(minutes=5),
expires_at=expires_at,
)
logger = FakeLogger() logger = FakeLogger()
create_usecase = FakeCreateSandboxUsecase(error=SandboxStartError('chat-123')) create_usecase = FakeCreateSandboxUsecase(session=session)
cleanup_usecase = FakeCleanupExpiredSandboxes() cleanup_usecase = FakeCleanupExpiredSandboxes()
docker_client = FakeDockerClient() docker_client = FakeDockerClient()
container = build_container( container = build_container(
@ -320,7 +332,37 @@ def test_post_create_maps_start_errors_to_service_unavailable(monkeypatch) -> No
app = app_module.create_app(config=config) app = app_module.create_app(config=config)
status_code, response = asyncio.run( status_code, response = asyncio.run(
exercise_create_request(app, {'chat_id': 'chat-123'}) exercise_create_request(app, {'chat_id': 'x/../y'})
)
assert status_code == 422
assert 'detail' in response
assert create_usecase.commands == []
assert docker_client.close_calls == 1
def test_post_create_maps_start_errors_to_service_unavailable(monkeypatch) -> None:
config = build_config()
logger = FakeLogger()
create_usecase = FakeCreateSandboxUsecase(error=SandboxStartError(CHAT_ID))
cleanup_usecase = FakeCleanupExpiredSandboxes()
docker_client = FakeDockerClient()
container = build_container(
config,
create_usecase,
cleanup_usecase,
logger,
docker_client,
)
monkeypatch.setattr(app_module, 'build_container', lambda **kwargs: container)
monkeypatch.setattr(
app_module.FastAPIInstrumentor, 'instrument_app', lambda *args, **kwargs: None
)
app = app_module.create_app(config=config)
status_code, response = asyncio.run(
exercise_create_request(app, {'chat_id': CHAT_ID})
) )
assert status_code == 503 assert status_code == 503
@ -349,7 +391,7 @@ def test_post_create_maps_generic_sandbox_errors_to_internal_error(monkeypatch)
app = app_module.create_app(config=config) app = app_module.create_app(config=config)
status_code, response = asyncio.run( status_code, response = asyncio.run(
exercise_create_request(app, {'chat_id': 'chat-123'}) exercise_create_request(app, {'chat_id': CHAT_ID})
) )
assert status_code == 500 assert status_code == 500
@ -362,7 +404,7 @@ def test_removed_user_endpoint_returns_not_found(monkeypatch) -> None:
expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC) expires_at = datetime(2026, 4, 2, 12, 5, tzinfo=UTC)
session = SandboxSession( session = SandboxSession(
session_id='session-123', session_id='session-123',
chat_id='chat-123', chat_id=CHAT_ID,
container_id='container-123', container_id='container-123',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=expires_at - timedelta(minutes=5), created_at=expires_at - timedelta(minutes=5),

View file

@ -12,6 +12,9 @@ from adapter.docker.runtime import DockerSandboxRuntime
from domain.error import SandboxError, SandboxStartError from domain.error import SandboxError, SandboxStartError
from domain.sandbox import SandboxStatus from domain.sandbox import SandboxStatus
CHAT_ID = '123e4567-e89b-12d3-a456-426614174000'
NON_CANONICAL_CHAT_ID = '123E4567E89B12D3A456426614174000'
class FakeContainer: class FakeContainer:
def __init__(self, container_id: str) -> None: def __init__(self, container_id: str) -> None:
@ -92,7 +95,9 @@ def build_config(tmp_path: Path) -> SandboxConfig:
) )
def test_runtime_create_applies_mount_policy_and_labels(tmp_path: Path) -> None: def test_runtime_create_applies_mount_policy_and_labels_with_canonical_chat_id(
tmp_path: Path,
) -> None:
config = build_config(tmp_path) config = build_config(tmp_path)
(tmp_path / 'dependencies').mkdir() (tmp_path / 'dependencies').mkdir()
(tmp_path / 'lambda-tools').mkdir() (tmp_path / 'lambda-tools').mkdir()
@ -103,25 +108,25 @@ def test_runtime_create_applies_mount_policy_and_labels(tmp_path: Path) -> None:
session = runtime.create( session = runtime.create(
session_id='session-123', session_id='session-123',
chat_id='chat-123', chat_id=NON_CANONICAL_CHAT_ID,
created_at=created_at, created_at=created_at,
expires_at=expires_at, expires_at=expires_at,
) )
assert session.session_id == 'session-123' assert session.session_id == 'session-123'
assert session.chat_id == 'chat-123' assert session.chat_id == CHAT_ID
assert session.container_id == 'container-123' assert session.container_id == 'container-123'
assert session.status is SandboxStatus.RUNNING assert session.status is SandboxStatus.RUNNING
assert session.created_at == created_at assert session.created_at == created_at
assert session.expires_at == expires_at assert session.expires_at == expires_at
assert (tmp_path / 'chats' / 'chat-123').is_dir() assert (tmp_path / 'chats' / CHAT_ID).is_dir()
call = containers.run_calls[0] call = containers.run_calls[0]
assert call['args'] == ('sandbox:latest',) assert call['args'] == ('sandbox:latest',)
assert call['kwargs']['detach'] is True assert call['kwargs']['detach'] is True
assert call['kwargs']['labels'] == { assert call['kwargs']['labels'] == {
'session_id': 'session-123', 'session_id': 'session-123',
'chat_id': 'chat-123', 'chat_id': CHAT_ID,
'expires_at': expires_at.isoformat(), 'expires_at': expires_at.isoformat(),
} }
@ -129,7 +134,7 @@ def test_runtime_create_applies_mount_policy_and_labels(tmp_path: Path) -> None:
assert [dict(mount) for mount in mounts] == [ assert [dict(mount) for mount in mounts] == [
{ {
'Target': '/workspace/chat', 'Target': '/workspace/chat',
'Source': str((tmp_path / 'chats' / 'chat-123').resolve(strict=False)), 'Source': str((tmp_path / 'chats' / CHAT_ID).resolve(strict=False)),
'Type': 'bind', 'Type': 'bind',
'ReadOnly': False, 'ReadOnly': False,
}, },
@ -160,13 +165,13 @@ def test_runtime_create_raises_start_error_when_container_id_is_missing(
with pytest.raises(SandboxStartError) as excinfo: with pytest.raises(SandboxStartError) as excinfo:
runtime.create( runtime.create(
session_id='session-123', session_id='session-123',
chat_id='chat-123', chat_id=CHAT_ID,
created_at=datetime(2026, 4, 2, 12, 0, tzinfo=UTC), created_at=datetime(2026, 4, 2, 12, 0, tzinfo=UTC),
expires_at=datetime(2026, 4, 2, 12, 5, tzinfo=UTC), expires_at=datetime(2026, 4, 2, 12, 5, tzinfo=UTC),
) )
assert str(excinfo.value) == 'sandbox_start_failed' assert str(excinfo.value) == 'sandbox_start_failed'
assert excinfo.value.chat_id == 'chat-123' assert excinfo.value.chat_id == CHAT_ID
def test_runtime_stop_ignores_missing_container(tmp_path: Path) -> None: def test_runtime_stop_ignores_missing_container(tmp_path: Path) -> None:
@ -192,7 +197,8 @@ def test_runtime_stop_wraps_docker_errors(tmp_path: Path) -> None:
assert str(excinfo.value) == 'sandbox_stop_failed' assert str(excinfo.value) == 'sandbox_stop_failed'
def test_runtime_create_rejects_chat_path_traversal(tmp_path: Path) -> None: @pytest.mark.parametrize('chat_id', ['.', 'a/..', 'x/../y'])
def test_runtime_create_rejects_non_uuid_chat_id(tmp_path: Path, chat_id: str) -> None:
config = build_config(tmp_path) config = build_config(tmp_path)
(tmp_path / 'dependencies').mkdir() (tmp_path / 'dependencies').mkdir()
(tmp_path / 'lambda-tools').mkdir() (tmp_path / 'lambda-tools').mkdir()
@ -202,11 +208,11 @@ def test_runtime_create_rejects_chat_path_traversal(tmp_path: Path) -> None:
with pytest.raises(SandboxStartError) as excinfo: with pytest.raises(SandboxStartError) as excinfo:
runtime.create( runtime.create(
session_id='session-123', session_id='session-123',
chat_id='../escape', chat_id=chat_id,
created_at=datetime(2026, 4, 2, 12, 0, tzinfo=UTC), created_at=datetime(2026, 4, 2, 12, 0, tzinfo=UTC),
expires_at=datetime(2026, 4, 2, 12, 5, tzinfo=UTC), expires_at=datetime(2026, 4, 2, 12, 5, tzinfo=UTC),
) )
assert str(excinfo.value) == 'sandbox_start_failed' assert str(excinfo.value) == 'sandbox_start_failed'
assert excinfo.value.chat_id == '../escape' assert excinfo.value.chat_id == chat_id
assert containers.run_calls == [] assert containers.run_calls == []

View file

@ -6,6 +6,14 @@ from repository.sandbox_lock import ProcessLocalSandboxLifecycleLocker
from repository.sandbox_session import InMemorySandboxSessionRepository from repository.sandbox_session import InMemorySandboxSessionRepository
from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand from usecase.sandbox import CleanupExpiredSandboxes, CreateSandbox, CreateSandboxCommand
CHAT_ID = '11111111-1111-1111-1111-111111111111'
NON_CANONICAL_CHAT_ID = '11111111111111111111111111111111'
EXPIRED_CHAT_ID = '22222222-2222-2222-2222-222222222222'
BOUNDARY_CHAT_ID = '33333333-3333-3333-3333-333333333333'
ACTIVE_CHAT_ID = '44444444-4444-4444-4444-444444444444'
FAIL_CHAT_ID = '55555555-5555-5555-5555-555555555555'
CLEAN_CHAT_ID = '66666666-6666-6666-6666-666666666666'
class FakeClock: class FakeClock:
def __init__(self, now: datetime) -> None: def __init__(self, now: datetime) -> None:
@ -183,7 +191,7 @@ def test_create_sandbox_reuses_active_session_when_not_expired() -> None:
now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC)
session = SandboxSession( session = SandboxSession(
session_id='session-1', session_id='session-1',
chat_id='chat-1', chat_id=CHAT_ID,
container_id='container-1', container_id='container-1',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now - timedelta(minutes=1), created_at=now - timedelta(minutes=1),
@ -203,19 +211,19 @@ def test_create_sandbox_reuses_active_session_when_not_expired() -> None:
ttl=timedelta(minutes=5), ttl=timedelta(minutes=5),
) )
result = usecase.execute(CreateSandboxCommand(chat_id='chat-1')) result = usecase.execute(CreateSandboxCommand(chat_id=CHAT_ID))
assert result == session assert result == session
assert runtime.create_calls == [] assert runtime.create_calls == []
assert runtime.stop_calls == [] assert runtime.stop_calls == []
assert repository.get_active_by_chat_id('chat-1') == session assert repository.get_active_by_chat_id(CHAT_ID) == session
assert locker.chat_ids == ['chat-1'] assert locker.chat_ids == [CHAT_ID]
assert logger.messages == [ assert logger.messages == [
( (
'info', 'info',
'sandbox_reused', 'sandbox_reused',
{ {
'chat_id': 'chat-1', 'chat_id': CHAT_ID,
'session_id': 'session-1', 'session_id': 'session-1',
'container_id': 'container-1', 'container_id': 'container-1',
}, },
@ -229,7 +237,7 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one(
now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC)
expired_session = SandboxSession( expired_session = SandboxSession(
session_id='session-old', session_id='session-old',
chat_id='chat-1', chat_id=CHAT_ID,
container_id='container-old', container_id='container-old',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now - timedelta(minutes=10), created_at=now - timedelta(minutes=10),
@ -250,33 +258,33 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one(
) )
monkeypatch.setattr('usecase.sandbox._new_session_id', lambda: 'session-new') monkeypatch.setattr('usecase.sandbox._new_session_id', lambda: 'session-new')
result = usecase.execute(CreateSandboxCommand(chat_id='chat-1')) result = usecase.execute(CreateSandboxCommand(chat_id=CHAT_ID))
assert runtime.stop_calls == ['container-old'] assert runtime.stop_calls == ['container-old']
assert runtime.create_calls == [ assert runtime.create_calls == [
{ {
'session_id': 'session-new', 'session_id': 'session-new',
'chat_id': 'chat-1', 'chat_id': CHAT_ID,
'created_at': now, 'created_at': now,
'expires_at': now + timedelta(minutes=5), 'expires_at': now + timedelta(minutes=5),
} }
] ]
assert result == SandboxSession( assert result == SandboxSession(
session_id='session-new', session_id='session-new',
chat_id='chat-1', chat_id=CHAT_ID,
container_id='container-session-new', container_id='container-session-new',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now, created_at=now,
expires_at=now + timedelta(minutes=5), expires_at=now + timedelta(minutes=5),
) )
assert repository.get_active_by_chat_id('chat-1') == result assert repository.get_active_by_chat_id(CHAT_ID) == result
assert locker.chat_ids == ['chat-1'] assert locker.chat_ids == [CHAT_ID]
assert logger.messages == [ assert logger.messages == [
( (
'info', 'info',
'sandbox_replaced', 'sandbox_replaced',
{ {
'chat_id': 'chat-1', 'chat_id': CHAT_ID,
'session_id': 'session-old', 'session_id': 'session-old',
'container_id': 'container-old', 'container_id': 'container-old',
}, },
@ -285,7 +293,7 @@ def test_create_sandbox_replaces_expired_session_and_creates_new_one(
'info', 'info',
'sandbox_created', 'sandbox_created',
{ {
'chat_id': 'chat-1', 'chat_id': CHAT_ID,
'session_id': 'session-new', 'session_id': 'session-new',
'container_id': 'container-session-new', 'container_id': 'container-session-new',
}, },
@ -308,9 +316,9 @@ def test_create_sandbox_creates_new_session_when_none_exists() -> None:
ttl=timedelta(minutes=5), ttl=timedelta(minutes=5),
) )
result = usecase.execute(CreateSandboxCommand(chat_id='chat-1')) result = usecase.execute(CreateSandboxCommand(chat_id=NON_CANONICAL_CHAT_ID))
assert result.chat_id == 'chat-1' assert result.chat_id == CHAT_ID
assert result.container_id == f'container-{result.session_id}' assert result.container_id == f'container-{result.session_id}'
assert result.status is SandboxStatus.RUNNING assert result.status is SandboxStatus.RUNNING
assert result.created_at == now assert result.created_at == now
@ -318,19 +326,20 @@ def test_create_sandbox_creates_new_session_when_none_exists() -> None:
assert len(runtime.create_calls) == 1 assert len(runtime.create_calls) == 1
assert runtime.create_calls[0] == { assert runtime.create_calls[0] == {
'session_id': result.session_id, 'session_id': result.session_id,
'chat_id': 'chat-1', 'chat_id': CHAT_ID,
'created_at': now, 'created_at': now,
'expires_at': now + timedelta(minutes=5), 'expires_at': now + timedelta(minutes=5),
} }
assert runtime.stop_calls == [] assert runtime.stop_calls == []
assert repository.get_active_by_chat_id('chat-1') == result assert repository.get_active_by_chat_id(CHAT_ID) == result
assert locker.chat_ids == ['chat-1'] assert repository.get_active_by_chat_id(NON_CANONICAL_CHAT_ID) is None
assert locker.chat_ids == [CHAT_ID]
assert logger.messages == [ assert logger.messages == [
( (
'info', 'info',
'sandbox_created', 'sandbox_created',
{ {
'chat_id': 'chat-1', 'chat_id': CHAT_ID,
'session_id': result.session_id, 'session_id': result.session_id,
'container_id': result.container_id, 'container_id': result.container_id,
}, },
@ -361,7 +370,7 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id(
def run_create(index: int) -> None: def run_create(index: int) -> None:
try: try:
results[index] = usecase.execute(CreateSandboxCommand(chat_id='chat-1')) results[index] = usecase.execute(CreateSandboxCommand(chat_id=CHAT_ID))
except Exception as exc: except Exception as exc:
errors.append(exc) errors.append(exc)
@ -384,7 +393,7 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id(
assert results[0] == results[1] assert results[0] == results[1]
assert results[0] == SandboxSession( assert results[0] == SandboxSession(
session_id='session-new', session_id='session-new',
chat_id='chat-1', chat_id=CHAT_ID,
container_id='container-session-new', container_id='container-session-new',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now, created_at=now,
@ -392,14 +401,14 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id(
) )
assert len(runtime.create_calls) == 1 assert len(runtime.create_calls) == 1
assert runtime.stop_calls == [] assert runtime.stop_calls == []
assert repository.get_active_by_chat_id('chat-1') == results[0] assert repository.get_active_by_chat_id(CHAT_ID) == results[0]
assert locker.chat_ids == ['chat-1', 'chat-1'] assert locker.chat_ids == [CHAT_ID, CHAT_ID]
assert logger.messages == [ assert logger.messages == [
( (
'info', 'info',
'sandbox_created', 'sandbox_created',
{ {
'chat_id': 'chat-1', 'chat_id': CHAT_ID,
'session_id': 'session-new', 'session_id': 'session-new',
'container_id': 'container-session-new', 'container_id': 'container-session-new',
}, },
@ -408,7 +417,7 @@ def test_create_sandbox_serializes_duplicate_concurrent_create_for_chat_id(
'info', 'info',
'sandbox_reused', 'sandbox_reused',
{ {
'chat_id': 'chat-1', 'chat_id': CHAT_ID,
'session_id': 'session-new', 'session_id': 'session-new',
'container_id': 'container-session-new', 'container_id': 'container-session-new',
}, },
@ -420,7 +429,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() ->
now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC)
expired_session = SandboxSession( expired_session = SandboxSession(
session_id='session-expired', session_id='session-expired',
chat_id='chat-expired', chat_id=EXPIRED_CHAT_ID,
container_id='container-expired', container_id='container-expired',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now - timedelta(minutes=10), created_at=now - timedelta(minutes=10),
@ -428,7 +437,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() ->
) )
boundary_session = SandboxSession( boundary_session = SandboxSession(
session_id='session-boundary', session_id='session-boundary',
chat_id='chat-boundary', chat_id=BOUNDARY_CHAT_ID,
container_id='container-boundary', container_id='container-boundary',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now - timedelta(minutes=5), created_at=now - timedelta(minutes=5),
@ -436,7 +445,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() ->
) )
active_session = SandboxSession( active_session = SandboxSession(
session_id='session-active', session_id='session-active',
chat_id='chat-active', chat_id=ACTIVE_CHAT_ID,
container_id='container-active', container_id='container-active',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now - timedelta(minutes=1), created_at=now - timedelta(minutes=1),
@ -461,16 +470,16 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() ->
assert result == [expired_session, boundary_session] assert result == [expired_session, boundary_session]
assert runtime.stop_calls == ['container-expired', 'container-boundary'] assert runtime.stop_calls == ['container-expired', 'container-boundary']
assert repository.get_active_by_chat_id('chat-expired') is None assert repository.get_active_by_chat_id(EXPIRED_CHAT_ID) is None
assert repository.get_active_by_chat_id('chat-boundary') is None assert repository.get_active_by_chat_id(BOUNDARY_CHAT_ID) is None
assert repository.get_active_by_chat_id('chat-active') == active_session assert repository.get_active_by_chat_id(ACTIVE_CHAT_ID) == active_session
assert locker.chat_ids == ['chat-expired', 'chat-boundary'] assert locker.chat_ids == [EXPIRED_CHAT_ID, BOUNDARY_CHAT_ID]
assert logger.messages == [ assert logger.messages == [
( (
'info', 'info',
'sandbox_cleaned', 'sandbox_cleaned',
{ {
'chat_id': 'chat-expired', 'chat_id': EXPIRED_CHAT_ID,
'session_id': 'session-expired', 'session_id': 'session-expired',
'container_id': 'container-expired', 'container_id': 'container-expired',
}, },
@ -479,7 +488,7 @@ def test_cleanup_expired_sandboxes_stops_and_deletes_only_expired_sessions() ->
'info', 'info',
'sandbox_cleaned', 'sandbox_cleaned',
{ {
'chat_id': 'chat-boundary', 'chat_id': BOUNDARY_CHAT_ID,
'session_id': 'session-boundary', 'session_id': 'session-boundary',
'container_id': 'container-boundary', 'container_id': 'container-boundary',
}, },
@ -491,7 +500,7 @@ def test_cleanup_expired_sandboxes_skips_replaced_session_from_stale_snapshot()
now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC)
expired_snapshot = SandboxSession( expired_snapshot = SandboxSession(
session_id='session-expired', session_id='session-expired',
chat_id='chat-1', chat_id=CHAT_ID,
container_id='container-expired', container_id='container-expired',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now - timedelta(minutes=10), created_at=now - timedelta(minutes=10),
@ -499,7 +508,7 @@ def test_cleanup_expired_sandboxes_skips_replaced_session_from_stale_snapshot()
) )
replacement_session = SandboxSession( replacement_session = SandboxSession(
session_id='session-new', session_id='session-new',
chat_id='chat-1', chat_id=CHAT_ID,
container_id='container-new', container_id='container-new',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now - timedelta(seconds=30), created_at=now - timedelta(seconds=30),
@ -522,8 +531,8 @@ def test_cleanup_expired_sandboxes_skips_replaced_session_from_stale_snapshot()
assert result == [] assert result == []
assert runtime.stop_calls == [] assert runtime.stop_calls == []
assert repository.get_active_by_chat_id('chat-1') == replacement_session assert repository.get_active_by_chat_id(CHAT_ID) == replacement_session
assert locker.chat_ids == ['chat-1'] assert locker.chat_ids == [CHAT_ID]
assert logger.messages == [] assert logger.messages == []
@ -531,7 +540,7 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None:
now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC) now = datetime(2026, 4, 2, 12, 0, tzinfo=UTC)
failing_session = SandboxSession( failing_session = SandboxSession(
session_id='session-fail', session_id='session-fail',
chat_id='chat-fail', chat_id=FAIL_CHAT_ID,
container_id='container-fail', container_id='container-fail',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now - timedelta(minutes=10), created_at=now - timedelta(minutes=10),
@ -539,7 +548,7 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None:
) )
cleaned_session = SandboxSession( cleaned_session = SandboxSession(
session_id='session-clean', session_id='session-clean',
chat_id='chat-clean', chat_id=CLEAN_CHAT_ID,
container_id='container-clean', container_id='container-clean',
status=SandboxStatus.RUNNING, status=SandboxStatus.RUNNING,
created_at=now - timedelta(minutes=9), created_at=now - timedelta(minutes=9),
@ -563,15 +572,15 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None:
assert result == [cleaned_session] assert result == [cleaned_session]
assert runtime.stop_calls == ['container-fail', 'container-clean'] assert runtime.stop_calls == ['container-fail', 'container-clean']
assert repository.get_active_by_chat_id('chat-fail') == failing_session assert repository.get_active_by_chat_id(FAIL_CHAT_ID) == failing_session
assert repository.get_active_by_chat_id('chat-clean') is None assert repository.get_active_by_chat_id(CLEAN_CHAT_ID) is None
assert locker.chat_ids == ['chat-fail', 'chat-clean'] assert locker.chat_ids == [FAIL_CHAT_ID, CLEAN_CHAT_ID]
assert logger.messages == [ assert logger.messages == [
( (
'error', 'error',
'sandbox_clean_failed', 'sandbox_clean_failed',
{ {
'chat_id': 'chat-fail', 'chat_id': FAIL_CHAT_ID,
'session_id': 'session-fail', 'session_id': 'session-fail',
'container_id': 'container-fail', 'container_id': 'container-fail',
'error': 'RuntimeError', 'error': 'RuntimeError',
@ -581,7 +590,7 @@ def test_cleanup_expired_sandboxes_continues_after_stop_failure() -> None:
'info', 'info',
'sandbox_cleaned', 'sandbox_cleaned',
{ {
'chat_id': 'chat-clean', 'chat_id': CLEAN_CHAT_ID,
'session_id': 'session-clean', 'session_id': 'session-clean',
'container_id': 'container-clean', 'container_id': 'container-clean',
}, },

View file

@ -1,6 +1,6 @@
from dataclasses import dataclass from dataclasses import dataclass
from datetime import timedelta from datetime import timedelta
from uuid import uuid4 from uuid import UUID, uuid4
from domain.sandbox import SandboxSession from domain.sandbox import SandboxSession
from usecase.interface import ( from usecase.interface import (
@ -35,15 +35,17 @@ class CreateSandbox:
self._ttl = ttl self._ttl = ttl
def execute(self, command: CreateSandboxCommand) -> SandboxSession: def execute(self, command: CreateSandboxCommand) -> SandboxSession:
with self._locker.lock(command.chat_id): chat_id = _canonical_chat_id(command.chat_id)
session = self._repository.get_active_by_chat_id(command.chat_id)
with self._locker.lock(chat_id):
session = self._repository.get_active_by_chat_id(chat_id)
now = self._clock.now() now = self._clock.now()
if session is not None and session.expires_at > now: if session is not None and session.expires_at > now:
self._logger.info( self._logger.info(
'sandbox_reused', 'sandbox_reused',
attrs={ attrs={
'chat_id': command.chat_id, 'chat_id': chat_id,
'session_id': session.session_id, 'session_id': session.session_id,
'container_id': session.container_id, 'container_id': session.container_id,
}, },
@ -54,7 +56,7 @@ class CreateSandbox:
self._logger.info( self._logger.info(
'sandbox_replaced', 'sandbox_replaced',
attrs={ attrs={
'chat_id': command.chat_id, 'chat_id': chat_id,
'session_id': session.session_id, 'session_id': session.session_id,
'container_id': session.container_id, 'container_id': session.container_id,
}, },
@ -66,7 +68,7 @@ class CreateSandbox:
expires_at = created_at + self._ttl expires_at = created_at + self._ttl
new_session = self._runtime.create( new_session = self._runtime.create(
session_id=_new_session_id(), session_id=_new_session_id(),
chat_id=command.chat_id, chat_id=chat_id,
created_at=created_at, created_at=created_at,
expires_at=expires_at, expires_at=expires_at,
) )
@ -74,7 +76,7 @@ class CreateSandbox:
self._logger.info( self._logger.info(
'sandbox_created', 'sandbox_created',
attrs={ attrs={
'chat_id': command.chat_id, 'chat_id': chat_id,
'session_id': new_session.session_id, 'session_id': new_session.session_id,
'container_id': new_session.container_id, 'container_id': new_session.container_id,
}, },
@ -151,3 +153,7 @@ class CleanupExpiredSandboxes:
def _new_session_id() -> str: def _new_session_id() -> str:
return uuid4().hex return uuid4().hex
def _canonical_chat_id(chat_id: str) -> str:
return str(UUID(str(chat_id).strip()))