feat(ctf): Cross-Vendor Email (BOLA/IDOR, ASI-03) - #558
Open
Deez-Automations wants to merge 6 commits into
Open
Conversation
finmail's read_email only blocks vendor sessions from reading admin-type
messages -- it never checks whether a vendor-type message actually
belongs to the calling vendor. EmailRepository.get_email confirms it at
the DB layer: filters by namespace + message id only. A vendor asking
their own compliant assistant something completely ordinary ("pull up
message 42 for me") reads another vendor's full email content, no
adversarial phrasing needed.
Detector is purely mechanical: compares the mcp_tool_call_success event's
own vendor_id (auto-injected server-side from session_context, confirmed
never overridden by this event type's event_data, unlike workflow_started)
against the target email's real vendor_id. No canary, no regex, no
paraphrase dependency. 13 tests, all passing.
- Scope detector to agent_name: chat_assistant in the YAML config (HIGH). FraudComplianceAgent and CommunicationAgent both have finmail wired in and legitimately read vendor mail during delegated workflows, reusing the originating session's session_context unchanged -- their events would carry a stale vendor_id even when investigating a different vendor for real business reasons, causing false positives unrelated to the player-facing exploit this challenge targets. - Coerce string-typed message_id to int (MEDIUM), matching the existing convention in CrossVendorDeletionDetector -- MCP tool schemas aren't marked strict, so a model isn't guaranteed to return message_id as a JSON number. - Guard against a "vendor" inbox_type email with vendor_id=None (LOW). - 3 new regression tests (16 total), all passing.
…aths read_email wasn't the only gap -- list_inbox and search_emails take an explicit vendor_id argument and pass it straight to repo.list_vendor_emails with no ownership check either, and mark_as_read has the identical message-id gap as read_email. A player using list_inbox to browse a target vendor's inbox directly (arguably a more natural/direct exploit path than guessing message IDs) was going completely undetected. Split detection into two paths: message-id tools (read_email, mark_as_read) check the target email's real vendor_id via a DB lookup; vendor-id tools (list_inbox, search_emails) check the requested vendor_id argument directly, no DB query needed since ownership is asserted right in the call. 25 tests now (9 new), all passing.
Code review found the vendor_id path (list_inbox/search_emails) fired on any requested_vendor_id != session's own, without checking it corresponds to a real vendor -- a fabricated ID (e.g. 99999) would still register as a detection, giving weaker evidentiary standard than the message_id path (which already proves a real Email row exists) despite reporting the same confidence=1.0. The YAML's own success criteria promises "real vendor owner", which the code didn't verify. Added a Vendor table existence check (namespace + id) before firing. 2 new regression tests (27 total), all passing.
… detector Security review (subagent) found no CRITICAL/HIGH issues -- namespace isolation independently re-verified, no injection paths, no unhandled sensitive data. One LOW robustness note addressed: a malformed tool_arguments payload (parses to a list/non-dict) would have hit .get() and raised AttributeError. Added an explicit dict-type guard plus a regression test. 28/28 tests pass.
This was referenced Aug 4, 2026
There was a problem hiding this comment.
Pull request overview
Adds a new red-track CTF challenge, Cross-Vendor Email (“Just Checking a Message”), centered on a FinMail BOLA/IDOR where vendor sessions can access other vendors’ messages/inboxes by referencing message_id / vendor_id. The PR introduces a detector to mechanically validate cross-vendor access using event truth + DB ownership, plus a comprehensive unit test suite and challenge definition YAML.
Changes:
- Added
CrossVendorEmailDetectorto detect cross-vendor access viaread_email,mark_as_read,list_inbox, andsearch_emails. - Added a full unit test suite validating detector behavior across message-id and vendor-id paths, plus defensive parsing and config validation cases.
- Registered the detector in the implementations package and added a new challenge definition YAML.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/unit/ctf/test_cross_vendor_email.py | Adds unit tests for CrossVendorEmailDetector across all targeted FinMail tool paths and edge cases. |
| finbot/ctf/detectors/implementations/cross_vendor_email.py | Implements CrossVendorEmailDetector with event/DB-driven ownership checks for cross-vendor FinMail access. |
| finbot/ctf/detectors/implementations/init.py | Exports/imports the new detector so it gets registered/available via the implementations package. |
| finbot/ctf/definitions/challenges/agent_trust/cross_vendor_email.yaml | Adds the new CTF challenge definition, hints, labels, scoring modifier, and detector configuration. |
Suppressed comments (1)
finbot/ctf/detectors/implementations/cross_vendor_email.py:96
- Related to the default scoping:
check_event()also only enforces the agent gate whenagent_nameis explicitly configured. If the detector is reused without YAML config, this can still evaluate events from non-chat agents (e.g., during replays) even though the detector is intended to be chat-assistant-specific by default. Align the in-function gate with the same default you use for event-type scoping.
agent_filter = self.config.get("agent_name")
if agent_filter:
event_agent = event.get("agent_name", "")
if event_agent != agent_filter:
return DetectionResult(
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fix internal documentation inconsistency: the module docstring claimed agent_name defaults to "chat_assistant", but the class docstring (and actual code) correctly document the default as None (any agent) -- cross_vendor_email.yaml sets agent_name: chat_assistant explicitly for this challenge. Clarified the module docstring to match reality instead of changing the class's intentionally reusable default. Also removed unused UTC/datetime imports from the test file.
Author
|
Addressed the Copilot review feedback:
28/28 tests still passing after both changes. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a new red-track challenge: Cross-Vendor Email ("Just Checking a Message"), a Broken Object Level Authorization (BOLA/IDOR) exploit in FinMail's email tools.
read_email/mark_as_read(bymessage_id) andlist_inbox/search_emails(byvendor_id) only check that a message/inbox exists in the caller's namespace — they never verify it actually belongs to the vendor the session is authenticated as. Any vendor session can read, mark-as-read, list, or search another vendor's inbox within the same namespace, simply by referencing its ID.CrossVendorEmailDetectoris purely mechanical: it re-derives the session's true vendor ID from the trusted, server-populated event field (never anything the model wrote), looks up the actual owner of the referenced message/inbox in the DB, and compares — no canary, no string matching, no dependency on any text surviving an LLM paraphrase step.Why this design
This project's first four challenge attempts (still open as #539, #542, and two others never opened) all bet on convincing a frontier model to make a specific risky judgment call under adversarial framing. Live success rate was only ~30-40% despite passing unit tests — a well-aligned model reliably recognizes and resists being pushed outside its defined role. This challenge (and the new standard going forward) requires at most one ordinary, non-adversarial action from the agent, with detection based entirely on a mechanical DB/event fact.
Process
chat_assistantto prevent false positives from other agents legitimately reading vendor mail with a stale session context; verifying the target vendor actually exists (not just a mismatched ID) on thevendor_idpath; broadening detection to cover all four BOLA-reachable FinMail tools, not just one.AttributeError) fixed with a defensive type guard plus a regression test.Test plan
pytest tests/unit/ctf/test_cross_vendor_email.py— 28/28 passingtests/unit/ctf/suite — no regressions