Skip to content

fix(finmail): cap email recipient count - #552

Open
DevaanshPathak wants to merge 1 commit into
GenAI-Security-Project:mainfrom
DevaanshPathak:fix/issue-346-finmail-recipient-limit
Open

fix(finmail): cap email recipient count#552
DevaanshPathak wants to merge 1 commit into
GenAI-Security-Project:mainfrom
DevaanshPathak:fix/issue-346-finmail-recipient-limit

Conversation

@DevaanshPathak

Copy link
Copy Markdown

Summary

  • cap FinMail delivery at 20 recipients across the combined to, cc, and bcc lists
  • reject oversized messages before serialization, vendor lookups, or email inserts
  • add regression coverage for oversized, aggregate-list, and exact-boundary behavior

Root cause

route_and_deliver iterated over every supplied recipient without enforcing an upper bound, allowing a single request to trigger an arbitrary number of database lookups and inserts.

Testing

  • uv run pytest tests/unit/mcp
  • uv run black --check tests/unit/mcp/test_finmail_routing.py
  • uv run isort --check-only finbot/mcp/servers/finmail/routing.py tests/unit/mcp/test_finmail_routing.py

Fixes #346

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a hard cap on FinMail delivery fanout by limiting the total number of recipients across the combined to, cc, and bcc lists to 20, preventing unbounded DB lookups/inserts from a single request.

Changes:

  • Add MAX_RECIPIENTS = 20 and enforce an early recipient-count check in route_and_deliver before any JSON serialization or database work.
  • Add unit tests covering oversized recipient lists, aggregate to+cc+bcc counting, and exact-boundary behavior at the limit.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
finbot/mcp/servers/finmail/routing.py Adds MAX_RECIPIENTS and an early return with an error when total recipients exceed the cap.
tests/unit/mcp/test_finmail_routing.py Adds regression tests ensuring oversize requests short-circuit before DB/repo calls and that the cap counts to, cc, and bcc together.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug_137_MUST_FIX: MCP-FM-ADDR-003 — 100-address recipient list accepted; unbounded delivery loop

2 participants