fix(mcp): persist vendor_account on FinStripe transfers - #563
Open
Deez-Automations wants to merge 1 commit into
Open
Conversation
…ty-Project#328) create_transfer echoed vendor_account back in its own response, but PaymentTransaction had no column for it at all -- the value was never actually stored. get_transfer and list_transfers always returned nothing for it. The real destination bank account was only ever visible in the immediate tool-call response, making it unrecoverable from the audit trail for investigating a misdirected payment. Adds the missing column, persists it in create_transaction, and includes a proper Alembic migration (verified applying cleanly against a real schema, chained after the current head). Only one call site exists for create_transaction in the whole codebase and it already uses keyword arguments exclusively, so inserting the new parameter mid-signature is safe. Fixes GenAI-Security-Project#328
There was a problem hiding this comment.
Pull request overview
This PR fixes a FinStripe MCP audit gap where vendor_account was returned by create_transfer but never persisted, causing get_transfer and list_transfers to omit the destination account when retrieving transfers later.
Changes:
- Add
vendor_accountcolumn to thePaymentTransactionmodel and include it into_dict(). - Persist
vendor_accountviaPaymentTransactionRepository.create_transactionand pass it through fromcreate_transfer. - Add an Alembic migration and new unit tests covering
get_transferandlist_transfersbehavior.
Reviewed changes
Copilot reviewed 5 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/mcp/test_finstripe.py | Adds unit tests that reproduce the persistence bug and verify vendor_account is retrievable via get_transfer and list_transfers. |
| migrations/versions/2026_08_11_add_vendor_account_to_payment_transactions.py | Adds Alembic migration to create the vendor_account column on payment_transactions. |
| finbot/mcp/servers/finstripe/server.py | Passes vendor_account into transaction creation during create_transfer. |
| finbot/mcp/servers/finstripe/repositories.py | Updates create_transaction to accept and persist vendor_account. |
| finbot/mcp/servers/finstripe/models.py | Adds vendor_account column and includes it in serialized transfer output. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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
Fixes #328.
create_transferechoedvendor_accountback in its own response, butPaymentTransactionhad no column for it at all — the value was never actually stored anywhere.get_transferandlist_transfers(both looking the same transaction back up) always returned nothing for it. The real destination bank account was only ever visible in the immediate tool-call response, making it unrecoverable from the audit trail — a genuine forensic gap for investigating a misdirected payment.Fix
vendor_accountcolumn toPaymentTransactionPaymentTransactionRepository.create_transactionnow accepts and persists itcreate_transferpasses it throughto_dict()now includes it, soget_transferandlist_transfersreturn the real, persisted valuea3f7c2d91e04)Checked that
create_transactionhas exactly one call site in the whole codebase, and it already uses keyword arguments exclusively — inserting the new parameter mid-signature carries no risk of silently misaligning a positional caller.Test plan
tests/unit/mcp/test_finstripe.py— reproduces the exact issue repro steps (create a transfer, look it back up, confirm the account is missing) against the unfixed code first, then confirms the fixlist_transfersreturning the persisted accountpytest tests/unit/mcp/test_finstripe.py— 2/2 passing