fix(email): enforce MCP owner boundaries (#4335)

* fix(email): enforce MCP owner boundaries

* fix(email): fail closed for unowned MCP fallback

tests/test_codex_ssh_host_validation.py

@@ -10,14 +10,14 @@ the validators the rest of the cookbook routes already apply.
 import asyncio
 
 import pytest
-from fastapi import HTTPException
+from fastapi import APIRouter, HTTPException
 from starlette.requests import Request
 
 import routes.codex_routes as codex_routes
 
 
-def _route_endpoint(path: str, method: str):
-    router = codex_routes.setup_codex_routes()
+def _route_endpoint(path: str, method: str, router=None):
+    router = router or codex_routes.setup_codex_routes()
     for route in router.routes:
         if route.path == path and method in route.methods:
             return route.endpoint
@@ -40,6 +40,22 @@ def _launch_request() -> Request:
     return request
 
 
+def _codex_request(scopes) -> Request:
+    request = Request(
+        {
+            "type": "http",
+            "method": "POST",
+            "path": "/api/codex/emails/draft-document",
+            "headers": [],
+            "state": {},
+        }
+    )
+    request.state.api_token = True
+    request.state.api_token_owner = "alice"
+    request.state.api_token_scopes = list(scopes)
+    return request
+
+
 def test_rejects_remote_host_with_shell_metacharacters():
     task = {"remoteHost": "box; rm -rf ~", "sshPort": ""}
     with pytest.raises(HTTPException) as exc:
@@ -105,3 +121,33 @@ def test_adopt_rejects_ssh_option_host_before_shell(monkeypatch):
 
     assert exc.value.status_code == 400
     assert calls == []
+
+
+@pytest.mark.asyncio
+async def test_email_draft_document_accepts_send_scope_with_document_write():
+    calls = []
+    document_router = APIRouter()
+
+    @document_router.post("/api/document")
+    async def create_document(request: Request, req):
+        calls.append((request.state.current_user, req.title, req.language, req.content))
+        return {"id": "doc-1", "title": req.title}
+
+    router = codex_routes.setup_codex_routes(document_router=document_router)
+    endpoint = _route_endpoint("/api/codex/emails/draft-document", "POST", router=router)
+
+    result = await endpoint(
+        _codex_request(["email:send", "documents:write"]),
+        {"to": "recipient@example.com", "subject": "Subject", "body": "Body"},
+    )
+
+    assert result["draft_type"] == "document"
+    assert result["send_required_confirmation"] is True
+    assert calls == [
+        (
+            "alice",
+            "Subject",
+            "email",
+            "To: recipient@example.com\nSubject: Subject\n---\nBody\n",
+        )
+    ]

tests/test_email_owner_scope.py

@@ -406,6 +406,54 @@ async def test_scheduled_email_routes_are_owner_scoped(tmp_path, monkeypatch):
     assert alice_rows["scheduled"] == []
 
 
+@pytest.mark.asyncio
+async def test_pending_agent_draft_routes_do_not_expose_ownerless_rows(tmp_path, monkeypatch):
+    import routes.email_helpers as email_helpers
+    import routes.email_routes as email_routes
+
+    db_path = tmp_path / "scheduled_emails.db"
+    monkeypatch.setattr(email_helpers, "SCHEDULED_DB", db_path)
+    monkeypatch.setattr(email_routes, "SCHEDULED_DB", db_path)
+    email_helpers._init_scheduled_db()
+
+    conn = sqlite3.connect(db_path)
+    conn.executemany(
+        """
+        INSERT INTO scheduled_emails
+        (id, to_addr, subject, body, attachments, send_at, created_at, status, account_id, owner)
+        VALUES (?, ?, ?, ?, '[]', '9999-12-31T00:00:00', ?, 'agent_draft', ?, ?)
+        """,
+        [
+            ("draft-ownerless", "nobody@example.com", "Ownerless", "old", "2026-01-01", "acct-a", ""),
+            ("draft-bob", "bob@example.com", "Bob", "bob body", "2026-01-02", "acct-b", "bob"),
+        ],
+    )
+    conn.commit()
+    conn.close()
+
+    router = email_routes.setup_email_routes()
+    list_pending = _route_endpoint(router, "/api/email/pending", "GET")
+    approve_pending = _route_endpoint(router, "/api/email/pending/{sid}/approve", "POST")
+    cancel_pending = _route_endpoint(router, "/api/email/pending/{sid}", "DELETE")
+
+    alice_rows = await list_pending(owner="alice")
+    bob_rows = await list_pending(owner="bob")
+
+    assert alice_rows["pending"] == []
+    assert [row["id"] for row in bob_rows["pending"]] == ["draft-bob"]
+    assert (await approve_pending("draft-ownerless", owner="alice"))["success"] is False
+    assert (await cancel_pending("draft-ownerless", owner="bob"))["success"] is False
+
+    conn = sqlite3.connect(db_path)
+    try:
+        rows = conn.execute(
+            "SELECT id, status FROM scheduled_emails ORDER BY id",
+        ).fetchall()
+    finally:
+        conn.close()
+    assert rows == [("draft-bob", "agent_draft"), ("draft-ownerless", "agent_draft")]
+
+
 def test_scheduled_poller_resolves_config_with_row_owner(tmp_path, monkeypatch):
     import routes.email_helpers as email_helpers
     import routes.email_pollers as email_pollers

tests/test_function_call_non_object_args.py

@@ -53,6 +53,13 @@ def test_non_object_arguments_do_not_crash(arguments):
     assert block.content == ""
 
 
+@pytest.mark.parametrize("tool_name", ["list_emails", "mcp__email__list_emails"])
+def test_email_mcp_non_object_arguments_are_rejected(tool_name):
+    block = function_call_to_tool_block(tool_name, '["INBOX"]')
+
+    assert block is None
+
+
 def test_edit_document_skips_non_object_edit_items():
     block = function_call_to_tool_block(
         "edit_document",

tests/test_mcp_email_decode_header_spaces.py

@@ -6,6 +6,9 @@ double space after "Re:" on every non-ASCII subject, a spurious space in
 "Name <addr>" senders, and violated RFC 2047 6.2 which requires whitespace
 between two adjacent encoded-words to be dropped.
 """
+import json
+import sqlite3
+
 import pytest
 
 pytest.importorskip("mcp")
@@ -13,6 +16,49 @@ pytest.importorskip("mcp")
 import mcp_servers.email_server as es
 
 
+def _init_accounts_db(path):
+    conn = sqlite3.connect(path)
+    conn.execute(
+        """
+        CREATE TABLE email_accounts (
+            id TEXT PRIMARY KEY,
+            owner TEXT,
+            name TEXT NOT NULL,
+            is_default INTEGER NOT NULL DEFAULT 0,
+            enabled INTEGER NOT NULL DEFAULT 1,
+            imap_host TEXT,
+            imap_port INTEGER,
+            imap_user TEXT,
+            imap_password TEXT,
+            imap_starttls INTEGER,
+            smtp_host TEXT,
+            smtp_port INTEGER,
+            smtp_security TEXT,
+            smtp_user TEXT,
+            smtp_password TEXT,
+            from_address TEXT,
+            created_at TEXT
+        )
+        """
+    )
+    conn.executemany(
+        """
+        INSERT INTO email_accounts
+        (id, owner, name, is_default, enabled, imap_host, imap_port, imap_user,
+         imap_password, imap_starttls, smtp_host, smtp_port, smtp_security,
+         smtp_user, smtp_password, from_address, created_at)
+        VALUES (?, ?, ?, ?, 1, 'imap.example.com', 993, ?, '', 1,
+                'smtp.example.com', 465, 'ssl', ?, '', ?, ?)
+        """,
+        [
+            ("acct-alice", "alice", "Alice Mail", 1, "alice@example.com", "alice@example.com", "alice@example.com", "2026-01-01"),
+            ("acct-bob", "bob", "Bob Mail", 1, "bob@example.com", "bob@example.com", "bob@example.com", "2026-01-02"),
+        ],
+    )
+    conn.commit()
+    conn.close()
+
+
 def test_prefix_then_encoded_word_single_space():
     assert es._decode_header("Re: =?utf-8?b?SsOzc2U=?=") == "Re: J\u00f3se"
 
@@ -32,3 +78,139 @@ def test_plain_ascii_header_unchanged():
 
 def test_empty_header():
     assert es._decode_header("") == ""
+
+
+@pytest.mark.asyncio
+async def test_mcp_email_accounts_are_filtered_by_hidden_owner(tmp_path, monkeypatch):
+    db_path = tmp_path / "app.db"
+    _init_accounts_db(db_path)
+    monkeypatch.setattr(es, "APP_DB", str(db_path))
+    es._ACCOUNT_CACHE.clear()
+
+    out = await es.call_tool("list_email_accounts", {"_odysseus_owner": "alice"})
+    text = out[0].text
+
+    assert "Alice Mail" in text
+    assert "Bob Mail" not in text
+
+
+@pytest.mark.asyncio
+async def test_mcp_email_requires_owner_when_multiple_account_owners_exist(tmp_path, monkeypatch):
+    db_path = tmp_path / "app.db"
+    _init_accounts_db(db_path)
+    monkeypatch.setattr(es, "APP_DB", str(db_path))
+    es._ACCOUNT_CACHE.clear()
+
+    out = await es.call_tool("list_email_accounts", {})
+
+    assert "requires an authenticated owner" in out[0].text
+
+
+def test_mcp_email_scoped_owner_without_visible_account_skips_legacy_fallback(tmp_path, monkeypatch):
+    db_path = tmp_path / "app.db"
+    settings_path = tmp_path / "settings.json"
+    _init_accounts_db(db_path)
+    settings_path.write_text(
+        json.dumps(
+            {
+                "imap_host": "legacy-imap.example.com",
+                "imap_user": "legacy@example.com",
+                "imap_password": "legacy-secret",
+                "smtp_host": "legacy-smtp.example.com",
+                "smtp_user": "legacy@example.com",
+                "smtp_password": "legacy-secret",
+                "from_address": "legacy@example.com",
+            }
+        ),
+        encoding="utf-8",
+    )
+    monkeypatch.setattr(es, "APP_DB", str(db_path))
+    monkeypatch.setattr(es, "_SETTINGS_FILE", str(settings_path))
+    es._ACCOUNT_CACHE.clear()
+
+    token = es._CURRENT_OWNER.set("charlie")
+    try:
+        with pytest.raises(ValueError, match="No email account is configured"):
+            es._load_config()
+    finally:
+        es._CURRENT_OWNER.reset(token)
+        es._ACCOUNT_CACHE.clear()
+
+
+@pytest.mark.asyncio
+async def test_mcp_send_email_stages_owner_scoped_pending_draft(tmp_path, monkeypatch):
+    import src.constants as constants
+
+    db_path = tmp_path / "scheduled_emails.db"
+    monkeypatch.setattr(constants, "SCHEDULED_EMAILS_DB", str(db_path))
+    monkeypatch.setattr(es, "_read_agent_email_confirm_setting", lambda: True)
+
+    out = await es.call_tool(
+        "send_email",
+        {
+            "to": "recipient@example.com",
+            "subject": "Review",
+            "body": "Please review.",
+            "_odysseus_owner": "alice",
+        },
+    )
+
+    assert "Draft staged for approval" in out[0].text
+    assert "Nothing has been sent yet" in out[0].text
+    conn = sqlite3.connect(db_path)
+    try:
+        row = conn.execute(
+            "SELECT owner, status, to_addr, subject FROM scheduled_emails"
+        ).fetchone()
+    finally:
+        conn.close()
+    assert row == ("alice", "agent_draft", "recipient@example.com", "Review")
+
+
+@pytest.mark.asyncio
+async def test_mcp_draft_email_document_uses_hidden_owner(monkeypatch):
+    import core.database as db_mod
+
+    saved = []
+
+    class FakeDocument:
+        def __init__(self, **kwargs):
+            self.__dict__.update(kwargs)
+
+    class FakeDocumentVersion:
+        def __init__(self, **kwargs):
+            self.__dict__.update(kwargs)
+
+    class FakeDb:
+        def add(self, obj):
+            saved.append(obj)
+
+        def commit(self):
+            pass
+
+        def close(self):
+            pass
+
+    monkeypatch.setattr(db_mod, "Document", FakeDocument)
+    monkeypatch.setattr(db_mod, "DocumentVersion", FakeDocumentVersion)
+    monkeypatch.setattr(db_mod, "SessionLocal", lambda: FakeDb())
+    monkeypatch.setattr(
+        es,
+        "_load_config",
+        lambda account=None: {"account_name": "Alice Mail", "account_id": "acct-alice"},
+    )
+
+    out = await es.call_tool(
+        "draft_email",
+        {
+            "to": "recipient@example.com",
+            "subject": "Draft subject",
+            "body": "Draft body",
+            "_odysseus_owner": "alice",
+        },
+    )
+
+    assert "Created Odysseus email draft" in out[0].text
+    docs = [obj for obj in saved if isinstance(obj, FakeDocument)]
+    assert len(docs) == 1
+    assert docs[0].owner == "alice"

tests/test_review_regressions.py

@@ -626,6 +626,63 @@ async def test_public_agent_policy_blocks_sensitive_tools(monkeypatch):
         assert "restricted to admin users" in result["error"]
 
 
+@pytest.mark.asyncio
+async def test_email_mcp_non_object_args_fail_before_dispatch(monkeypatch):
+    import src.tool_execution as tool_execution
+    from src.tool_execution import execute_tool_block
+
+    class FakeMcp:
+        def __init__(self):
+            self.calls = []
+
+        async def call_tool(self, name, args):
+            self.calls.append((name, args))
+            return {"output": "called", "exit_code": 0}
+
+    fake = FakeMcp()
+    monkeypatch.setattr(tool_execution, "_owner_is_admin", lambda owner: True)
+    monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: fake)
+
+    desc, result = await execute_tool_block(
+        SimpleNamespace(tool_type="mcp__email__list_emails", content='["INBOX"]'),
+        owner="alice",
+    )
+
+    assert desc == "mcp: mcp__email__list_emails"
+    assert result["exit_code"] == 1
+    assert "JSON object" in result["error"]
+    assert fake.calls == []
+
+
+@pytest.mark.asyncio
+async def test_email_mcp_dispatch_includes_hidden_owner(monkeypatch):
+    import src.tool_execution as tool_execution
+    from src.tool_execution import execute_tool_block
+
+    class FakeMcp:
+        def __init__(self):
+            self.calls = []
+
+        async def call_tool(self, name, args):
+            self.calls.append((name, args))
+            return {"output": "called", "exit_code": 0}
+
+    fake = FakeMcp()
+    monkeypatch.setattr(tool_execution, "_owner_is_admin", lambda owner: True)
+    monkeypatch.setattr(tool_execution, "get_mcp_manager", lambda: fake)
+
+    desc, result = await execute_tool_block(
+        SimpleNamespace(tool_type="mcp__email__list_emails", content='{"folder":"INBOX"}'),
+        owner="alice",
+    )
+
+    assert desc == "mcp: mcp__email__list_emails"
+    assert result["exit_code"] == 0
+    assert fake.calls == [
+        ("mcp__email__list_emails", {"folder": "INBOX", "_odysseus_owner": "alice"}),
+    ]
+
+
 def test_public_agent_policy_hides_sensitive_tools(monkeypatch):
     auth_mod = _install_core_auth_stub(monkeypatch)
     from src.tool_security import blocked_tools_for_owner