From 70215e200c4b5803f78cd96f45a1ce139a030956 Mon Sep 17 00:00:00 2001 From: Ali <010109ali@gmail.com> Date: Wed, 13 May 2026 16:52:34 +0000 Subject: [PATCH] fix(bank-integration): preview XSS, header completeness, ref-number safeguards, per-file currency MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Escape t.ref_no / t.counterparty / t.purpose / error fields with escape_html when rendering the preview and error-detail dialogs. A malicious bank statement (or counterparty-supplied purpose text) containing HTML could otherwise execute in the importer's session. - Parser now refuses to proceed if any standard field required by the selected Amount Mode is missing from the file headers — instead of silently dropping every row with reason "amount" / "date". - Reference Number is now a required mapping for all Amount Modes, preventing a class of silent duplication on re-import. parse_excel_for_ preview also returns empty_ref_count and the preview surfaces a warning about future duplicate risk when some rows have an empty Reference Number on disk. - Add 'Use Currency From File' checkbox on Excel Preset (default off). When on, BT.currency uses the parsed Currency column with fallback to the Bank Account's currency. import_bulk_bt now takes preset_name and the background job reads the flag. Co-Authored-By: Claude Opus 4.7 (1M context) --- jey_erp/bank_integration/excel_parser.py | 26 ++++++++++++++++- jey_erp/bank_integration/import_api.py | 22 +++++++++++++-- .../bank_integration_excel_preset.json | 8 ++++++ .../bank_integration_excel_preset.py | 6 ++-- jey_erp/public/js/bank_transaction_list.js | 28 +++++++++++++------ 5 files changed, 74 insertions(+), 16 deletions(-) diff --git a/jey_erp/bank_integration/excel_parser.py b/jey_erp/bank_integration/excel_parser.py index e10fca2..e24e021 100644 --- a/jey_erp/bank_integration/excel_parser.py +++ b/jey_erp/bank_integration/excel_parser.py @@ -25,6 +25,15 @@ STANDARD_FIELD_TO_CODE = { } +# Internal codes that must be present in the headers of any imported file, by +# amount_mode. Kept in sync with bank_integration_excel_preset.py:_REQUIRED_BY_MODE. +_REQUIRED_CODES_BY_MODE = { + "Single column with sign": ("date", "reference_number", "amount"), + "Separate debit/credit columns": ("date", "reference_number", "debit", "credit"), + "Single column + direction column": ("date", "reference_number", "amount", "direction"), +} + + # Tried in order until one parses the value. Covers ISO-style, day-first (EU/AZ), # and month-first (US) on the four common separators, with and without time. # Day-first variants are listed BEFORE month-first to bias towards the AZ region; @@ -105,9 +114,24 @@ def parse_excel(file_url, preset_name): f"found in Excel headers ({[h for h in headers if h]})" ) + amount_mode = preset.amount_mode or "Separate debit/credit columns" + found_codes = set(idx_to_standard.values()) + required = _REQUIRED_CODES_BY_MODE.get(amount_mode, ()) + missing_codes = [c for c in required if c not in found_codes] + if missing_codes: + # Reverse-map codes back to human labels for the message. + code_to_label = {v: k for k, v in STANDARD_FIELD_TO_CODE.items()} + missing_labels = [code_to_label.get(c, c) for c in missing_codes] + frappe.throw( + "Required column(s) for Amount Mode '{}' not found in the file headers: {}. " + "Either the Header Row on the preset points at the wrong line, or the Excel " + "column names in the preset don't match the file. Headers seen: {}".format( + amount_mode, ", ".join(missing_labels), [h for h in headers if h] + ) + ) + use_custom = bool(preset.get("use_custom_date_format")) custom_format = (preset.date_format or "").strip() if use_custom else "" - amount_mode = preset.amount_mode or "Separate debit/credit columns" debit_aliases = _split_aliases(preset.get("direction_debit_values")) credit_aliases = _split_aliases(preset.get("direction_credit_values")) diff --git a/jey_erp/bank_integration/import_api.py b/jey_erp/bank_integration/import_api.py index e4a3743..09bc1dd 100644 --- a/jey_erp/bank_integration/import_api.py +++ b/jey_erp/bank_integration/import_api.py @@ -19,6 +19,11 @@ def parse_excel_for_preview(file_url, preset_name, bank_integration, bank_accoun dropped_direction = result.get("dropped_direction", 0) unknown_directions = result.get("unknown_directions", []) + # Rows with an empty reference_number cannot be deduplicated against later + # imports — surface a warning so the user notices and fixes the source file + # or the Reference Number mapping in the preset. + empty_ref_count = sum(1 for t in transactions if not (t.get("ref_no") or "").strip()) + if not transactions: return { "success": True, @@ -29,6 +34,7 @@ def parse_excel_for_preview(file_url, preset_name, bank_integration, bank_accoun "dropped_amount": dropped_amount, "dropped_direction": dropped_direction, "unknown_directions": unknown_directions, + "empty_ref_count": 0, } ref_nos = [t["ref_no"] for t in transactions if t.get("ref_no")] @@ -62,11 +68,12 @@ def parse_excel_for_preview(file_url, preset_name, bank_integration, bank_accoun "dropped_amount": dropped_amount, "dropped_direction": dropped_direction, "unknown_directions": unknown_directions, + "empty_ref_count": empty_ref_count, } @frappe.whitelist() -def import_bulk_bt(txn_list, bank_integration, bank_account): +def import_bulk_bt(txn_list, bank_integration, bank_account, preset_name=None): """Enqueue bulk Bank Transaction creation as a background job. Creates Bank Transactions only — registries (Bank Integration Customer / @@ -84,6 +91,7 @@ def import_bulk_bt(txn_list, bank_integration, bank_account): txn_list=txn_list, bank_integration=bank_integration, bank_account=bank_account, + preset_name=preset_name, user=user, queue="default", timeout=600, @@ -92,7 +100,7 @@ def import_bulk_bt(txn_list, bank_integration, bank_account): return {"success": True, "enqueued": True, "total": len(txn_list)} -def _process_bulk_bt_import(txn_list, bank_integration, bank_account, user): +def _process_bulk_bt_import(txn_list, bank_integration, bank_account, user, preset_name=None): """Background job: create one Bank Transaction per row.""" frappe.set_user(user) @@ -105,6 +113,12 @@ def _process_bulk_bt_import(txn_list, bank_integration, bank_account, user): frappe.db.get_value("Account", ba_account, "account_currency") if ba_account else None ) or "AZN" + use_currency_from_file = False + if preset_name: + use_currency_from_file = bool( + frappe.db.get_value("Bank Integration Excel Preset", preset_name, "use_currency_from_file") + ) + total = len(txn_list) imported_count = 0 errors = [] @@ -123,11 +137,13 @@ def _process_bulk_bt_import(txn_list, bank_integration, bank_account, user): ref_no = (txn.get("ref_no") or "").strip() amount = abs(flt(txn.get("amount", 0))) + row_currency = (txn.get("currency") or "").strip().upper() if use_currency_from_file else "" + bt = frappe.new_doc("Bank Transaction") bt.date = txn.get("date") bt.deposit = amount if drcr == "C" else 0 bt.withdrawal = amount if drcr == "D" else 0 - bt.currency = ba_currency + bt.currency = row_currency or ba_currency bt.description = txn.get("description") or purpose bt.reference_number = ref_no bt.transaction_id = ref_no diff --git a/jey_erp/jey_erp/doctype/bank_integration_excel_preset/bank_integration_excel_preset.json b/jey_erp/jey_erp/doctype/bank_integration_excel_preset/bank_integration_excel_preset.json index 61551a8..cc15867 100644 --- a/jey_erp/jey_erp/doctype/bank_integration_excel_preset/bank_integration_excel_preset.json +++ b/jey_erp/jey_erp/doctype/bank_integration_excel_preset/bank_integration_excel_preset.json @@ -12,6 +12,7 @@ "amount_mode", "direction_debit_values", "direction_credit_values", + "use_currency_from_file", "column_break_1", "use_custom_date_format", "date_format", @@ -70,6 +71,13 @@ "fieldtype": "Small Text", "label": "Credit Values" }, + { + "default": "0", + "description": "Off: every imported Bank Transaction gets its Bank Account's currency.\nOn: the parsed Currency column on each row is used, falling back to the Bank Account's currency when empty. Enable only for multi-currency Bank Accounts where the statement actually carries per-row currency.", + "fieldname": "use_currency_from_file", + "fieldtype": "Check", + "label": "Use Currency From File" + }, { "fieldname": "column_break_1", "fieldtype": "Column Break" diff --git a/jey_erp/jey_erp/doctype/bank_integration_excel_preset/bank_integration_excel_preset.py b/jey_erp/jey_erp/doctype/bank_integration_excel_preset/bank_integration_excel_preset.py index f5e3f49..de54851 100644 --- a/jey_erp/jey_erp/doctype/bank_integration_excel_preset/bank_integration_excel_preset.py +++ b/jey_erp/jey_erp/doctype/bank_integration_excel_preset/bank_integration_excel_preset.py @@ -9,9 +9,9 @@ from frappe.model.document import Document # Standard fields the parser needs depending on the chosen amount_mode. # Date is always required. _REQUIRED_BY_MODE = { - "Single column with sign": ("Date", "Amount"), - "Separate debit/credit columns": ("Date", "Debit", "Credit"), - "Single column + direction column": ("Date", "Amount", "Direction"), + "Single column with sign": ("Date", "Reference Number", "Amount"), + "Separate debit/credit columns": ("Date", "Reference Number", "Debit", "Credit"), + "Single column + direction column": ("Date", "Reference Number", "Amount", "Direction"), } diff --git a/jey_erp/public/js/bank_transaction_list.js b/jey_erp/public/js/bank_transaction_list.js index ee019b6..9664411 100644 --- a/jey_erp/public/js/bank_transaction_list.js +++ b/jey_erp/public/js/bank_transaction_list.js @@ -119,6 +119,7 @@ const BIExcelImport = { const droppedAmount = r.message.dropped_amount || 0; const droppedDirection = r.message.dropped_direction || 0; const unknownDirections = r.message.unknown_directions || []; + const emptyRefCount = r.message.empty_ref_count || 0; if (droppedDate > 0 && !txns.length) { BIExcelImport._showDateParseHelp(droppedDate, values.preset); @@ -143,12 +144,12 @@ const BIExcelImport = { return; } BIExcelImport._showPreview(txns, skipped, values, listview, - droppedDate, droppedAmount, droppedDirection, unknownDirections); + droppedDate, droppedAmount, droppedDirection, unknownDirections, emptyRefCount); }, }); }, - _showPreview(txns, skipped, values, listview, droppedDate, droppedAmount, droppedDirection, unknownDirections) { + _showPreview(txns, skipped, values, listview, droppedDate, droppedAmount, droppedDirection, unknownDirections, emptyRefCount) { let table = '
'; if (skipped > 0) { table += '
' + @@ -174,6 +175,12 @@ const BIExcelImport = { __('Dropped {0} row(s) — missing or zero amount.', [droppedAmount]) + '
'; } + if (emptyRefCount > 0) { + table += '
' + + __('{0} of {1} row(s) have no Reference Number — re-importing the same file will create duplicates because deduplication uses the Reference Number.', + [emptyRefCount, txns.length]) + + '
'; + } table += ''; table += ''; table += ''; @@ -185,6 +192,7 @@ const BIExcelImport = { table += ''; table += ''; + const esc = frappe.utils.escape_html; txns.forEach((t, i) => { const dateStr = t.date ? moment(t.date).format('DD.MM.YYYY') : ''; const amountStr = parseFloat(t.amount || 0).toFixed(2); @@ -194,12 +202,12 @@ const BIExcelImport = { const text = t.purpose || t.description || ''; table += '' + '' + - '' + + '' + '' + - '' + + '' + '' + '' + - '' + + '' + ''; }); table += '
' + __('Purpose / Description') + '
' + (t.ref_no || '') + '' + esc(t.ref_no || '') + '' + dateStr + '' + (t.counterparty || '') + '' + esc(t.counterparty || '') + '' + amountStr + '' + drcrLabel + '' + text + '' + esc(text) + '
'; @@ -284,6 +292,7 @@ const BIExcelImport = { txn_list: JSON.stringify(selected), bank_integration: values.bank_integration, bank_account: values.bank_account, + preset_name: values.preset, }, error() { frappe.realtime.off('bi_bt_import_progress'); @@ -387,14 +396,15 @@ const BIExcelImport = { frappe.msgprint(__('No error details')); return; } + const esc = frappe.utils.escape_html; let html = '
'; errs.forEach(e => { html += '
'; - html += '' + (e.ref_no || 'No ref') + ''; - html += ' ' + (e.error_type || 'error') + ''; + html += '' + esc(e.ref_no || 'No ref') + ''; + html += ' ' + esc(e.error_type || 'error') + ''; html += '
' + - (e.counterparty || '') + ' — ' + parseFloat(e.amount || 0).toFixed(2) + '
'; - html += '
' + (e.message || '') + '
'; + esc(e.counterparty || '') + ' — ' + parseFloat(e.amount || 0).toFixed(2) + '
'; + html += '
' + esc(e.message || '') + '
'; html += '
'; }); html += '';