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>
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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"))
|
||||
if user_id is None:
|
||||
resolved = await resolve_bearer(headers.get("authorization"))
|
||||
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)
|
||||
|
||||
Reference in New Issue
Block a user