docs(security): reconcile assistant and threat contracts

This commit is contained in:
RaresKeY 2026-08-06 17:06:14 +01:00
commit 8b11ef55c7
4 changed files with 54 additions and 20 deletions

View file

@ -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.

View file

@ -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:

View file

@ -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

View file

@ -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