fix(ms365-app): require caller authentication on the chat-tool route

Closed
OP
8h ago
+366-3

Summary

POST /tools/{tool_name} in plugins/omi-ms365-app/main.py took uid from the request body and dispatched to a handler that acts on that user's stored Microsoft Graph token, with no caller authentication on the route:

@app.post("/tools/{tool_name}")
async def tool_dispatch(tool_name: str, request: Request) -> Any:
    ...
    uid: str | None = body.get("uid") or request.query_params.get("uid")
    ...
    await _auth_guard(uid)          # asserts the *victim* is connected, not that the caller may act
    return await handler(uid, **args)

_auth_guard only checks that a Microsoft token exists for the named uid, so naming a connected user is enough. The 17 registered tools include read_email, search_emails, send_email, list_recent_chats, send_chat_message, list_recent_files, read_file_text and upload_text_file, and config.GRAPH_SCOPES requests Mail.Read, Mail.Send, Mail.ReadWrite, Calendars.ReadWrite, Chat.ReadWrite and Files.ReadWrite.All. Any client that can reach the deployment and name a uid could read a user's mailbox and send mail as them.

Same defect class as the guards already merged for omi-github-app (github_tools_auth.py) and omi-shipbob-app (shipbob_tools_auth.py), and as the earlier zapier / paid-mentor / webhook fixes; ms365 was missed by all of them. /tools/* in this app is the most sensitive instance left: it is mail, not inventory.

Fix

New plugins/omi-ms365-app/ms365_tools_auth.py with require_ms365_tools_auth, mirroring the merged guards rather than inventing a second shape:

  • Shared secret from MS365_TOOLS_SECRET, presented as Authorization: Bearer <secret> or an ms365_tools_token query parameter.
  • hmac.compare_digest (timing-safe); a whitespace-only secret counts as unconfigured.
  • Fail-closed 503 when unconfigured, 401 on a missing, wrong or non-Bearer token.
  • Applied as dependencies=[Depends(require_ms365_tools_auth)] on the single chat-tool route. Caller authentication only — uid stays where it is, in the body.

Docs: MS365_TOOLS_SECRET added to the README's required-environment table, the endpoint table, the curl example, and .env.example, plus a short section stating that the same value has to reach the Omi backend or every tool call is refused.

plugins/omi-ms365-app/test_tool_dispatch.py and plugins/test_oauth_error_params_escaped.py import this main.py against fastapi stubs; both stubs gain a Depends attribute, which their decorator stubs then ignore.

Verification

  • python3 plugins/omi-ms365-app/test_ms365_tools_auth.py — 11 tests. Nine cover the guard's decision table (unconfigured→503, blank secret→503, no token→401, wrong bearer→401, wrong query token→401, Basic scheme→401, correct bearer→ok, bearer lowercase→ok, correct query token→ok); two load the real main.py through a recording stub and assert the chat-tool route — and only it — carries require_ms365_tools_auth, so the dependency cannot be dropped from the decorator unnoticed.
  • Also passes with fastapi blocked from import, matching this app's stdlib-only test lane.
  • Existing suites unchanged and passing: test_tool_dispatch.py (7), test_input_coercion.py (8), test_services_hardening.py, test_calendar_pagination.py, plugins/test_oauth_error_params_escaped.py (3), plugins/test_uid_reflection_remaining_apps.py (9).
  • make preflight — 21 checks pass, including the new ms365-tools-auth-tests entry registered in .github/checks-manifest.yaml (both lanes).

Failure-Class: none

🤖 Generated with Claude Code