Repository navigation
fix(transfer): run the bank-write validator on document import into an existing bank (#5137) - #5308
Merged
Merged
Conversation
…n existing bank (#5137) Importing documents only reached an operation-validator hook when the target bank did not exist yet (validate_create_bank inside _ensure_bank_exists). An import into an existing bank reached none, so a validator that confines API keys to their own bank never saw a key merging an archive into another bank. import_documents_async now calls validate_bank_write with a new BankWriteOperation.IMPORT_DOCUMENTS before anything else. It covers both routes that merge documents: POST .../document-transfer and POST .../transfer/import?mode=merge. Restore mode was already safe: it only writes a bank that does not exist, which validate_create_bank gates. Tests: - api-slim: an import into an existing, forbidden bank raises 403, the validator sees IMPORT_DOCUMENTS, and nothing is imported. - system story test_83: a bank:<id> key imports into its own bank and is refused on the neighbour's, through both routes; the neighbour stays empty. The test validator gains the bank:<id> rule for this.
This was referenced Oct 6, 2026
This was referenced Oct 6, 2026
Merged
This was referenced Oct 14, 2026
Merged
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.
Fixes #5137.
Problem
Importing documents into a bank that already exists never reached an operation-validator hook. The only check on that path was
validate_create_bank, which_ensure_bank_existsruns only when the bank is missing. So a validator that limits API keys to their own bank could not stop a key from merging an archive into another existing bank.Fix
import_documents_asyncnow callsvalidate_bank_writewith a newBankWriteOperation.IMPORT_DOCUMENTSbefore doing anything else. That one method serves both routes that merge documents:POST /v1/default/banks/{bank_id}/document-transfer(deprecated)POST /v1/default/banks/{bank_id}/transfer/import?mode=mergeRestore mode (
transfer/import, default) was already safe: it only writes a bank that does not exist yet, whichvalidate_create_bankgates.Tests
test_import_into_existing_bank_runs_bank_write_validator: importing into an existing, forbidden bank raises 403, the validator seesIMPORT_DOCUMENTS, and nothing is imported.test_83_bank_scoped_import: abank:<id>key imports into its own bank and is refused on the neighbour's bank through both routes; the neighbour stays empty. The test validator (server_extensions/tag_scope_validator.py) gains thebank:<id>rule for this.Both tests fail without the fix.
Out of scope
The issue also mentions
mental-models/{id}/history, which is already fixed on main by #3831. Separate operation names for export, audit logs and LLM requests would be a policy nice-to-have; those routes are already checked underGET_BANK_PROFILE.