From 8b11ef55c71679bb028ecfcc19ffc509fc51fc2e Mon Sep 17 00:00:00 2001 From: RaresKeY <158580472+RaresKeY@users.noreply.github.com> Date: Thu, 6 Aug 2026 17:06:14 +0100 Subject: [PATCH] docs(security): reconcile assistant and threat contracts --- THREAT_MODEL.md | 6 +-- routes/assistant_routes.py | 18 ++++----- src/task_scheduler.py | 13 ++++--- tests/test_current_assistant_security_docs.py | 37 +++++++++++++++++++ 4 files changed, 54 insertions(+), 20 deletions(-) create mode 100644 tests/test_current_assistant_security_docs.py diff --git a/THREAT_MODEL.md b/THREAT_MODEL.md index 48665a61d..c2143376a 100644 --- a/THREAT_MODEL.md +++ b/THREAT_MODEL.md @@ -74,8 +74,6 @@ These are open, acknowledged, and contributor help is welcome: 1. **No shell/filesystem sandbox.** The agent `bash` and `read_file`/`write_file` tools run as the app process user with no network egress filtering or filesystem confinement. A successful prompt-injection reaching a shell-enabled admin session can make outbound requests to internal services. See #1058 for the sandbox proposal. -2. **SSRF via `/api/v1/chat` `base_url` parameter.** A chat-scoped API token can supply an arbitrary `base_url`; the server forwards the LLM request to that host without validating the scheme or address. PR #1039 fixes this. +2. **API-token coverage is surface-specific.** Tokens have separate chat, todo, document, email, calendar, memory, and Cookbook scopes. Only routes that explicitly map the token owner and enforce the relevant scope are supported; a token is not a general subset of all UI/session privileges. Companion pairing currently mints a chat-scoped token. -3. **`src/search/` partial consolidation.** `src.search.core` and `src.search.providers` correctly alias `services.search` via `sys.modules` replacement. `analytics`, `cache`, `content`, `query`, and `ranking` are still independent copies that can drift. The SSRF regression tests in `tests/test_webhook_ssrf_resilience.py` test `src.webhook_manager` directly (separate from search), so the safety net there is intact. See #1058. - -4. **Token scopes are coarse.** There is no way to grant a session a subset of the owning user's privileges. Companion/mobile tokens carry either `chat` or `admin` scope with no per-capability granularity. +`POST /api/v1/chat` validates a token-supplied direct `base_url` with the public-HTTP URL policy before making a provider request. Admin-configured model endpoints intentionally retain local/LAN support and are a separate trust boundary. diff --git a/routes/assistant_routes.py b/routes/assistant_routes.py index 0b609e37f..1f8cbb6a1 100644 --- a/routes/assistant_routes.py +++ b/routes/assistant_routes.py @@ -1,10 +1,9 @@ """Personal assistant routes — resolve the per-user singleton, read/write its settings, and list its scheduled check-in tasks. -The personal assistant is just a specially-flagged CrewMember that owns one -pinned Session and three daily ScheduledTasks ("Morning/Midday/Evening -check-in"). Everything about it is user-editable: name, personality, model, -enabled tools, timezone, and the three check-in times/prompts/enabled flags. +The personal assistant is a specially-flagged CrewMember that owns one pinned +Session. Users can attach recurring check-in ScheduledTasks explicitly; those +tasks remain editable here, but assistant creation does not seed them. """ import json @@ -86,10 +85,9 @@ def setup_assistant_routes(task_scheduler) -> APIRouter: raise HTTPException(status_code=401, detail="Not authenticated") return owner - # Synthetic / non-human owners that should NEVER get an assistant + - # check-in tasks seeded. Hitting any /assistant route under one of these - # used to seed a full CrewMember + Morning/Midday/Evening tasks under that - # owner, which then double-fired alongside the real user's check-ins. + # Synthetic / non-human owners must never get a personal assistant. Older + # versions also seeded check-in tasks for these owners, which could + # double-fire alongside tasks belonging to the real user. # RESERVED_USERNAMES covers the same set; the `not owner` guard handles "". async def _get_or_create(owner: str) -> CrewMember: @@ -134,7 +132,7 @@ def setup_assistant_routes(task_scheduler) -> APIRouter: @router.get("/settings") async def get_assistant_settings(request: Request): - """Return CrewMember fields + the three check-in task rows + task IDs for logs.""" + """Return CrewMember fields and any user-configured check-in tasks.""" owner = _owner(request) crew = await _get_or_create(owner) if not crew: @@ -155,7 +153,7 @@ def setup_assistant_routes(task_scheduler) -> APIRouter: @router.patch("/settings") async def update_assistant_settings(payload: AssistantSettingsUpdate, request: Request): - """Update CrewMember fields and/or check-in tasks in one call.""" + """Update CrewMember fields and/or existing check-in tasks in one call.""" owner = _owner(request) crew = await _get_or_create(owner) if not crew: diff --git a/src/task_scheduler.py b/src/task_scheduler.py index d5b1dad62..ca00f76b2 100644 --- a/src/task_scheduler.py +++ b/src/task_scheduler.py @@ -2476,14 +2476,15 @@ class TaskScheduler: logger.warning(f"Failed to seed assistant for {owner}: {e}") async def ensure_assistant_defaults(self, owner: str): - """Create the personal-assistant CrewMember, its pinned session, and three - daily check-in ScheduledTasks for this owner — idempotent on is_default_assistant.""" + """Create the personal-assistant CrewMember and its pinned session. + + Check-in tasks are user-created and are not seeded here. Creation is + idempotent on ``is_default_assistant``. + """ # Hard-reject synthetic owners. Without this, AuthMiddleware-stamped # values like 'internal-tool' (loopback agent-tool callbacks) or 'api' - # (bearer-token integrations) would get a real assistant + 3 daily - # check-ins seeded, which then double-fire alongside the human user's - # check-ins. This was the root cause of the duplicate 'Morning check-in' - # rows we had to manually clean up. + # (bearer-token integrations) would get a real assistant. Older builds + # also seeded three daily check-ins for those synthetic owners. if not owner or owner in RESERVED_USERNAMES: logger.info(f"ensure_assistant_defaults: skip synthetic owner {owner!r}") return diff --git a/tests/test_current_assistant_security_docs.py b/tests/test_current_assistant_security_docs.py new file mode 100644 index 000000000..ba37290da --- /dev/null +++ b/tests/test_current_assistant_security_docs.py @@ -0,0 +1,37 @@ +from pathlib import Path + + +ROOT = Path(__file__).resolve().parents[1] + +TOKEN_SCOPE_PARAGRAPH = "2. **API-token coverage is surface-specific.** Tokens have separate chat, todo, document, email, calendar, memory, and Cookbook scopes. Only routes that explicitly map the token owner and enforce the relevant scope are supported; a token is not a general subset of all UI/session privileges. Companion pairing currently mints a chat-scoped token." +CHAT_URL_PARAGRAPH = "`POST /api/v1/chat` validates a token-supplied direct `base_url` with the public-HTTP URL policy before making a provider request. Admin-configured model endpoints intentionally retain local/LAN support and are a separate trust boundary." + + +def test_assistant_docs_do_not_claim_checkins_are_seeded(): + assistant = (ROOT / "routes" / "assistant_routes.py").read_text(encoding="utf-8") + scheduler = (ROOT / "src" / "task_scheduler.py").read_text(encoding="utf-8") + + assert "three daily ScheduledTasks" not in assistant + assert "daily check-in ScheduledTasks for this owner" not in scheduler + assert "Check-in tasks are user-created" in scheduler + + +def test_threat_model_matches_current_token_and_chat_url_boundaries(): + threat_model = (ROOT / "THREAT_MODEL.md").read_text(encoding="utf-8") + lines = threat_model.splitlines() + + assert "SSRF via `/api/v1/chat`" not in threat_model + assert TOKEN_SCOPE_PARAGRAPH in lines + assert CHAT_URL_PARAGRAPH in lines + assert "`src/search/` partial consolidation" not in threat_model + assert "still independent copies" not in threat_model + + +def test_search_compat_modules_point_at_the_canonical_service(): + for name in ("analytics.py", "cache.py", "content.py", "query.py"): + source = (ROOT / "src" / "search" / name).read_text(encoding="utf-8") + assert "from services.search" in source + assert "sys.modules[__name__]" in source + + ranking = (ROOT / "src" / "search" / "ranking.py").read_text(encoding="utf-8") + assert "from services.search.ranking import" in ranking