fix(contract-drift): MCP read-only scope, shared-note writes, event TZ

Drift-audit Group 5 (high-severity contract drift):

- MCP read-only keys could call every write tool: the Bearer resolver
  discarded api_key.scope and dispatch had no gate. Add resolve_bearer()
  (returns user_id + scope) and a scope gate in the /mcp ASGI wrapper that
  buffers the JSON-RPC body and rejects tools/call for any tool outside a
  read all-list when scope=='read' (default-deny for unknown/new tools).
- Shared project notes/tasks panel was empty for non-owners: get_project_notes_route
  now queries notes/milestones with the project OWNER's uid (mirrors the
  already-fixed milestones route).
- Shared editors couldn't save/delete shared NOTES (tasks worked): the three
  notes write routes now resolve via get_note_for_user, gate on can_write_note,
  and write as the owner — matching the tasks routes.
- Event timezone drift: naive datetimes from the MCP date+time split are now
  localized to the user's tz at a single canonical service point (create_event
  /update_event), so MCP- and UI-created events agree. tz-aware inputs
  (REST/CalDAV) pass through untouched.
- create_note validates status/priority (TaskStatus/TaskPriority), closing the
  MCP create_task path that let out-of-enum values persist (no DB CHECK).

Tests cover resolve_bearer scope + the write-tool classifier.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

src/fabledassistant/mcp/auth.py

@@ -17,3 +17,21 @@ async def resolve_bearer_to_user_id(auth_header: str | None) -> int | None:
         return None
     api_key = await lookup_key(raw_token)
     return api_key.user_id if api_key else None
+
+
+async def resolve_bearer(auth_header: str | None) -> tuple[int, str] | None:
+    """Resolve a Bearer token to (user_id, scope).
+
+    scope is 'read' or 'write'. Returns None for a missing/malformed/invalid
+    token. The MCP dispatch layer uses scope to deny write-class tool calls
+    from read-only keys — the same read/write boundary the REST API enforces.
+    """
+    if not auth_header or not auth_header.startswith("Bearer "):
+        return None
+    raw_token = auth_header[len("Bearer "):].strip()
+    if not raw_token:
+        return None
+    api_key = await lookup_key(raw_token)
+    if api_key is None:
+        return None
+    return api_key.user_id, (api_key.scope or "write")

src/fabledassistant/mcp/server.py

@@ -81,6 +81,66 @@ auto-purges after the operator's retention window.
 """
 
 
+# Tools a read-only API key may call. Anything not listed is treated as a
+# write for read keys (default-deny), so a newly-added tool is locked down
+# until explicitly classified here.
+_READ_ONLY_TOOLS = frozenset({
+    "get_event", "get_note", "get_project", "get_rule", "get_rulebook",
+    "get_task", "get_recent", "enter_project",
+    "list_events", "list_lists", "list_milestones", "list_notes",
+    "list_persons", "list_places", "list_projects", "list_rulebooks",
+    "list_rules", "list_tags", "list_tasks", "list_topics", "list_trash",
+    "list_always_on_rules", "search",
+})
+
+
+async def _buffer_request_body(receive):
+    """Drain the ASGI request body and return (body_bytes, replay_receive).
+
+    The MCP sub-app still needs to read the body, so we return a fresh
+    `receive` that replays the buffered bytes.
+    """
+    chunks: list[bytes] = []
+    more = True
+    while more:
+        message = await receive()
+        if message["type"] == "http.request":
+            chunks.append(message.get("body", b""))
+            more = message.get("more_body", False)
+        else:  # http.disconnect
+            more = False
+    body = b"".join(chunks)
+
+    sent = False
+
+    async def replay():
+        nonlocal sent
+        if not sent:
+            sent = True
+            return {"type": "http.request", "body": body, "more_body": False}
+        return {"type": "http.disconnect"}
+
+    return body, replay
+
+
+def _body_calls_write_tool(body: bytes) -> bool:
+    """True if the JSON-RPC body invokes a tool outside the read all-list."""
+    import json
+    try:
+        payload = json.loads(body)
+    except Exception:
+        return False
+    items = payload if isinstance(payload, list) else [payload]
+    for item in items:
+        if not isinstance(item, dict):
+            continue
+        if item.get("method") == "tools/call":
+            name = (item.get("params") or {}).get("name", "")
+            if name and name not in _READ_ONLY_TOOLS:
+                return True
+    return False
+
+
 def build_mcp_server() -> FastMCP:
     """Build the FastMCP instance with all tools registered.
 
@@ -128,7 +188,7 @@ def mount_mcp(app: Quart) -> None:
     inside Quart, we hook the session manager's `run()` async context manager
     into Quart's serving lifecycle (before_serving / after_serving).
     """
-    from fabledassistant.mcp.auth import resolve_bearer_to_user_id
+    from fabledassistant.mcp.auth import resolve_bearer
 
     mcp = build_mcp_server()
     mcp_asgi = mcp.streamable_http_app()
@@ -151,8 +211,8 @@ def mount_mcp(app: Quart) -> None:
             return await mcp_asgi(scope, receive, send)
         # ASGI headers are lowercase bytes per spec; lowercase explicitly to be safe.
         headers = {k.decode().lower(): v.decode() for k, v in scope.get("headers", [])}
-        user_id = await resolve_bearer_to_user_id(headers.get("authorization"))
+        resolved = await resolve_bearer(headers.get("authorization"))
-        if user_id is None:
+        if resolved is None:
             await send({
                 "type": "http.response.start",
                 "status": 401,
@@ -166,6 +226,27 @@ def mount_mcp(app: Quart) -> None:
                 "body": b'{"error":"unauthorized"}',
             })
             return
+        user_id, key_scope = resolved
+
+        # Enforce read-only keys: REST blocks non-GET for scope='read', and the
+        # MCP surface must match or the read-only guarantee is void. A tool call
+        # arrives as a JSON-RPC POST; buffer the body, and if it invokes a tool
+        # outside the read all-list, reject before dispatch. (default-deny: any
+        # unknown/new tool is treated as a write for read keys.)
+        if key_scope == "read" and scope.get("method") == "POST":
+            body, receive = await _buffer_request_body(receive)
+            if _body_calls_write_tool(body):
+                await send({
+                    "type": "http.response.start",
+                    "status": 403,
+                    "headers": [(b"content-type", b"application/json")],
+                })
+                await send({
+                    "type": "http.response.body",
+                    "body": b'{"error":"read-only API key cannot call write tools"}',
+                })
+                return
+
         scope["scribe_user_id"] = user_id
         from fabledassistant.mcp._context import _user_id_ctx
         token = _user_id_ctx.set(user_id)

src/fabledassistant/routes/notes.py

@@ -8,6 +8,7 @@ from quart import Blueprint, jsonify, request
 
 from fabledassistant.auth import login_required, get_current_user_id
 from fabledassistant.routes.utils import not_found, parse_iso_date, parse_pagination
+from fabledassistant.services.access import can_write_note
 from fabledassistant.services.notes import (
     build_note_graph,
     convert_note_to_task,
@@ -192,6 +193,15 @@ async def get_note_route(note_id: int):
 @login_required
 async def update_note_route(note_id: int):
     uid = get_current_user_id()
+    # Share-aware: resolve through the ACL and write as the OWNER, so a shared
+    # editor's save isn't rejected by the owner-scoped update service.
+    result = await get_note_for_user(uid, note_id)
+    if result is None:
+        return not_found("Note")
+    note_obj, _ = result
+    if not await can_write_note(uid, note_id):
+        return jsonify({"error": "Permission denied"}), 403
+    owner_uid = note_obj.user_id
     data = await request.get_json()
     fields = {}
     for key in ("title", "body", "description", "parent_id", "project_id", "milestone_id", "status", "priority", "note_type"):
@@ -212,14 +222,14 @@ async def update_note_route(note_id: int):
     if "tags" in data:
         fields["tags"] = data["tags"]
     try:
-        note = await update_note(uid, note_id, **fields)
+        note = await update_note(owner_uid, note_id, **fields)
     except ValueError as e:
         return jsonify({"error": str(e)}), 400
     if note is None:
         return not_found("Note")
     text = f"{note.title}\n{note.body}".strip() if note.body else (note.title or "")
     if text:
-        asyncio.create_task(upsert_note_embedding(note.id, uid, text))
+        asyncio.create_task(upsert_note_embedding(note.id, owner_uid, text))
     return jsonify(note.to_dict())
 
 
@@ -227,6 +237,13 @@ async def update_note_route(note_id: int):
 @login_required
 async def patch_note_route(note_id: int):
     uid = get_current_user_id()
+    result = await get_note_for_user(uid, note_id)
+    if result is None:
+        return not_found("Note")
+    note_obj, _ = result
+    if not await can_write_note(uid, note_id):
+        return jsonify({"error": "Permission denied"}), 403
+    owner_uid = note_obj.user_id
     data = await request.get_json()
     fields = {}
     for key in ("title", "body", "description", "parent_id", "project_id", "milestone_id", "status", "priority", "note_type"):
@@ -245,14 +262,14 @@ async def patch_note_route(note_id: int):
     if "tags" in data:
         fields["tags"] = data["tags"]
     try:
-        note = await update_note(uid, note_id, **fields)
+        note = await update_note(owner_uid, note_id, **fields)
     except ValueError as e:
         return jsonify({"error": str(e)}), 400
     if note is None:
         return not_found("Note")
     text = f"{note.title}\n{note.body}".strip() if note.body else (note.title or "")
     if text:
-        asyncio.create_task(upsert_note_embedding(note.id, uid, text))
+        asyncio.create_task(upsert_note_embedding(note.id, owner_uid, text))
     return jsonify(note.to_dict())
 
 
@@ -260,8 +277,14 @@ async def patch_note_route(note_id: int):
 @login_required
 async def delete_note_route(note_id: int):
     uid = get_current_user_id()
+    result = await get_note_for_user(uid, note_id)
+    if result is None:
+        return not_found("Note")
+    note_obj, _ = result
+    if not await can_write_note(uid, note_id):
+        return jsonify({"error": "Permission denied"}), 403
     from fabledassistant.services.trash import delete as trash_delete
-    batch = await trash_delete(uid, "note", note_id)
+    batch = await trash_delete(note_obj.user_id, "note", note_id)
     if batch is None:
         return not_found("Note")
     return "", 204

src/fabledassistant/routes/projects.py

@@ -116,6 +116,9 @@ async def get_project_notes_route(project_id: int):
     if result is None:
         return not_found("Project")
     project, _ = result
+    # Use the project owner's uid so the ownership filter on notes/milestones
+    # matches for shared collaborators (who'd otherwise see an empty panel).
+    owner_uid = project.user_id or uid
 
     # type filter: "note", "task", or None (both)
     type_filter = request.args.get("type")
@@ -128,11 +131,11 @@ async def get_project_notes_route(project_id: int):
     elif type_filter == "note":
         is_task = False
 
-    ms_list = await list_milestones(uid, project_id)
+    ms_list = await list_milestones(owner_uid, project_id)
     milestone_ids = [m.id for m in ms_list]
 
     notes, total = await list_notes(
-        uid,
+        owner_uid,
         is_task=is_task,
         status=status_filter,
         project_id=project_id,

src/fabledassistant/services/events.py

@@ -68,6 +68,19 @@ def _normalize_duration(
     return None
 
 
+async def _localize_naive(user_id: int, dt: datetime | None) -> datetime | None:
+    """Anchor a naive datetime in the user's timezone; pass tz-aware through.
+
+    Naive datetimes are the user's local wall-clock time (the MCP create/update
+    tools combine date+time without a zone). Attaching the user's tzinfo lets
+    asyncpg store the correct UTC instant, matching the REST/UI path.
+    """
+    if dt is not None and dt.tzinfo is None:
+        from fabledassistant.services.tz import get_user_tz  # noqa: PLC0415
+        return dt.replace(tzinfo=await get_user_tz(user_id))
+    return dt
+
+
 async def create_event(
     user_id: int,
     title: str,
@@ -97,6 +110,13 @@ async def create_event(
     """
     if duration is not None and duration_minutes is None:
         duration_minutes = duration
+    # Canonical localization point: a naive datetime (e.g. from the MCP tool's
+    # date+time split) is the user's wall-clock time, so anchor it in their
+    # timezone before storage. tz-aware inputs (REST, CalDAV pass-through) are
+    # left untouched. Without this, MCP-created events landed at the same
+    # wall-clock numerals in UTC and drifted from UI-created ones by the offset.
+    start_dt = await _localize_naive(user_id, start_dt)
+    end_dt = await _localize_naive(user_id, end_dt)
     duration_minutes = _normalize_duration(
         start_dt=start_dt, end_dt=end_dt, duration_minutes=duration_minutes,
     )
@@ -271,6 +291,11 @@ async def update_event(user_id: int, event_id: int, **fields) -> Event | None:
             return None
         old_title = event.title  # capture before mutation for CalDAV lookup
 
+        # Localize a naive start_dt patch to the user's timezone (same canonical
+        # rule as create_event) before it's used or persisted.
+        if fields.get("start_dt") is not None:
+            fields["start_dt"] = await _localize_naive(user_id, fields["start_dt"])
+
         # Resolve any end_dt/duration_minutes inputs against the
         # post-update start_dt. If neither is in the patch, leave the
         # existing duration_minutes alone.

src/fabledassistant/services/notes.py

@@ -66,6 +66,20 @@ async def create_note(
     entity_meta: dict | None = None,
     task_kind: str = "work",
 ) -> Note:
+    # Validate status/priority here so the MCP create_task path (which passes
+    # them straight through) can't persist an out-of-enum value that the REST
+    # route would have rejected — there's no DB CHECK on notes.status.
+    if isinstance(status, str):
+        try:
+            status = TaskStatus(status).value
+        except ValueError:
+            raise ValueError(f"Invalid status: {status!r}. Must be one of: {[s.value for s in TaskStatus]}")
+    if isinstance(priority, str):
+        try:
+            priority = TaskPriority(priority).value
+        except ValueError:
+            raise ValueError(f"Invalid priority: {priority!r}. Must be one of: {[p.value for p in TaskPriority]}")
+
     # Auto-populate project_id from milestone when not explicitly provided
     if milestone_id is not None and project_id is None:
         from fabledassistant.models.milestone import Milestone

tests/test_mcp_auth.py

@@ -49,3 +49,47 @@ async def test_resolve_bearer_calls_lookup_with_stripped_token():
     with patch("fabledassistant.mcp.auth.lookup_key", mock_lookup):
         await resolve_bearer_to_user_id("Bearer fmcp_abc123  ")
     mock_lookup.assert_awaited_once_with("fmcp_abc123")
+
+
+# ── resolve_bearer (user_id + scope) ────────────────────────────────────
+
+@pytest.mark.asyncio
+async def test_resolve_bearer_returns_user_id_and_scope():
+    from fabledassistant.mcp.auth import resolve_bearer
+    fake_key = MagicMock()
+    fake_key.user_id = 9
+    fake_key.scope = "read"
+    with patch("fabledassistant.mcp.auth.lookup_key", AsyncMock(return_value=fake_key)):
+        assert await resolve_bearer("Bearer fmcp_x") == (9, "read")
+
+
+@pytest.mark.asyncio
+async def test_resolve_bearer_none_for_invalid():
+    with patch("fabledassistant.mcp.auth.lookup_key", AsyncMock(return_value=None)):
+        assert await resolve_bearer("Bearer nope") is None
+    assert await resolve_bearer(None) is None
+
+
+# ── read-only scope gate ────────────────────────────────────────────────
+
+def test_body_calls_write_tool_classifies_correctly():
+    import json
+    from fabledassistant.mcp.server import _body_calls_write_tool
+
+    def call(name):
+        return json.dumps({"jsonrpc": "2.0", "id": 1, "method": "tools/call",
+                           "params": {"name": name, "arguments": {}}}).encode()
+
+    # Write-class tools are gated.
+    assert _body_calls_write_tool(call("create_note")) is True
+    assert _body_calls_write_tool(call("delete_project")) is True
+    assert _body_calls_write_tool(call("purge_trash")) is True
+    # An unknown/new tool defaults to write (default-deny for read keys).
+    assert _body_calls_write_tool(call("brand_new_tool")) is True
+    # Read tools and non-call methods pass.
+    assert _body_calls_write_tool(call("list_notes")) is False
+    assert _body_calls_write_tool(call("get_recent")) is False
+    assert _body_calls_write_tool(
+        json.dumps({"method": "tools/list"}).encode()
+    ) is False
+    assert _body_calls_write_tool(b"not json") is False