Skip to content

Commit 7f1a6f6

Browse files
codexRaresKeY
authored andcommitted
docs(security): reconcile assistant and threat contracts
1 parent 42da399 commit 7f1a6f6

4 files changed

Lines changed: 54 additions & 20 deletions

File tree

THREAT_MODEL.md

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -74,8 +74,6 @@ These are open, acknowledged, and contributor help is welcome:
7474

7575
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.
7676

77-
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.
77+
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.
7878

79-
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.
80-
81-
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.
79+
`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.

routes/assistant_routes.py

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,9 @@
11
"""Personal assistant routes — resolve the per-user singleton, read/write
22
its settings, and list its scheduled check-in tasks.
33
4-
The personal assistant is just a specially-flagged CrewMember that owns one
5-
pinned Session and three daily ScheduledTasks ("Morning/Midday/Evening
6-
check-in"). Everything about it is user-editable: name, personality, model,
7-
enabled tools, timezone, and the three check-in times/prompts/enabled flags.
4+
The personal assistant is a specially-flagged CrewMember that owns one pinned
5+
Session. Users can attach recurring check-in ScheduledTasks explicitly; those
6+
tasks remain editable here, but assistant creation does not seed them.
87
"""
98

109
import json
@@ -86,10 +85,9 @@ def _owner(request: Request) -> str:
8685
raise HTTPException(status_code=401, detail="Not authenticated")
8786
return owner
8887

89-
# Synthetic / non-human owners that should NEVER get an assistant +
90-
# check-in tasks seeded. Hitting any /assistant route under one of these
91-
# used to seed a full CrewMember + Morning/Midday/Evening tasks under that
92-
# owner, which then double-fired alongside the real user's check-ins.
88+
# Synthetic / non-human owners must never get a personal assistant. Older
89+
# versions also seeded check-in tasks for these owners, which could
90+
# double-fire alongside tasks belonging to the real user.
9391
# RESERVED_USERNAMES covers the same set; the `not owner` guard handles "".
9492

9593
async def _get_or_create(owner: str) -> CrewMember:
@@ -134,7 +132,7 @@ async def get_assistant_session(request: Request):
134132

135133
@router.get("/settings")
136134
async def get_assistant_settings(request: Request):
137-
"""Return CrewMember fields + the three check-in task rows + task IDs for logs."""
135+
"""Return CrewMember fields and any user-configured check-in tasks."""
138136
owner = _owner(request)
139137
crew = await _get_or_create(owner)
140138
if not crew:
@@ -155,7 +153,7 @@ async def get_assistant_settings(request: Request):
155153

156154
@router.patch("/settings")
157155
async def update_assistant_settings(payload: AssistantSettingsUpdate, request: Request):
158-
"""Update CrewMember fields and/or check-in tasks in one call."""
156+
"""Update CrewMember fields and/or existing check-in tasks in one call."""
159157
owner = _owner(request)
160158
crew = await _get_or_create(owner)
161159
if not crew:

src/task_scheduler.py

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2476,14 +2476,15 @@ def _score(candidate):
24762476
logger.warning(f"Failed to seed assistant for {owner}: {e}")
24772477

24782478
async def ensure_assistant_defaults(self, owner: str):
2479-
"""Create the personal-assistant CrewMember, its pinned session, and three
2480-
daily check-in ScheduledTasks for this owner — idempotent on is_default_assistant."""
2479+
"""Create the personal-assistant CrewMember and its pinned session.
2480+
2481+
Check-in tasks are user-created and are not seeded here. Creation is
2482+
idempotent on ``is_default_assistant``.
2483+
"""
24812484
# Hard-reject synthetic owners. Without this, AuthMiddleware-stamped
24822485
# values like 'internal-tool' (loopback agent-tool callbacks) or 'api'
2483-
# (bearer-token integrations) would get a real assistant + 3 daily
2484-
# check-ins seeded, which then double-fire alongside the human user's
2485-
# check-ins. This was the root cause of the duplicate 'Morning check-in'
2486-
# rows we had to manually clean up.
2486+
# (bearer-token integrations) would get a real assistant. Older builds
2487+
# also seeded three daily check-ins for those synthetic owners.
24872488
if not owner or owner in RESERVED_USERNAMES:
24882489
logger.info(f"ensure_assistant_defaults: skip synthetic owner {owner!r}")
24892490
return
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
from pathlib import Path
2+
3+
4+
ROOT = Path(__file__).resolve().parents[1]
5+
6+
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."
7+
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."
8+
9+
10+
def test_assistant_docs_do_not_claim_checkins_are_seeded():
11+
assistant = (ROOT / "routes" / "assistant_routes.py").read_text(encoding="utf-8")
12+
scheduler = (ROOT / "src" / "task_scheduler.py").read_text(encoding="utf-8")
13+
14+
assert "three daily ScheduledTasks" not in assistant
15+
assert "daily check-in ScheduledTasks for this owner" not in scheduler
16+
assert "Check-in tasks are user-created" in scheduler
17+
18+
19+
def test_threat_model_matches_current_token_and_chat_url_boundaries():
20+
threat_model = (ROOT / "THREAT_MODEL.md").read_text(encoding="utf-8")
21+
lines = threat_model.splitlines()
22+
23+
assert "SSRF via `/api/v1/chat`" not in threat_model
24+
assert TOKEN_SCOPE_PARAGRAPH in lines
25+
assert CHAT_URL_PARAGRAPH in lines
26+
assert "`src/search/` partial consolidation" not in threat_model
27+
assert "still independent copies" not in threat_model
28+
29+
30+
def test_search_compat_modules_point_at_the_canonical_service():
31+
for name in ("analytics.py", "cache.py", "content.py", "query.py"):
32+
source = (ROOT / "src" / "search" / name).read_text(encoding="utf-8")
33+
assert "from services.search" in source
34+
assert "sys.modules[__name__]" in source
35+
36+
ranking = (ROOT / "src" / "search" / "ranking.py").read_text(encoding="utf-8")
37+
assert "from services.search.ranking import" in ranking

0 commit comments

Comments
 (0)