From be1aa0e5ebcc05e3b3587333651a95aea55756cf Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Sun, 14 Jun 2026 23:41:59 +0530 Subject: [PATCH 1/2] fix: regression issues related to security fixes --- erpnext/accounts/party.py | 6 +- .../doctype/purchase_order/purchase_order.py | 2 +- .../subcontracting_inward_controller.py | 4 +- .../doctype/sales_order/sales_order.py | 2 +- .../stock/doctype/stock_entry/stock_entry.py | 6 +- .../subcontracting_inward_order.py | 10 +++- .../subcontracting_order.py | 10 +++- .../test_subcontracting_order.py | 56 +++++++++++++++++++ 8 files changed, 85 insertions(+), 11 deletions(-) diff --git a/erpnext/accounts/party.py b/erpnext/accounts/party.py index c70b6251ebb..15645602965 100644 --- a/erpnext/accounts/party.py +++ b/erpnext/accounts/party.py @@ -507,10 +507,10 @@ def get_party_advance_account(party_type, party, company): return account -@frappe.whitelist() def get_party_bank_account(party_type: str, party: str): - frappe.has_permission("Bank Account", "read", throw=True) - return frappe.db.get_value("Bank Account", {"party_type": party_type, "party": party, "is_default": 1}) + return frappe.db.get_value( + "Bank Account", {"party_type": party_type, "party": party, "is_default": 1, "disabled": 0}, "name" + ) def get_party_account_currency(party_type, party, company): diff --git a/erpnext/buying/doctype/purchase_order/purchase_order.py b/erpnext/buying/doctype/purchase_order/purchase_order.py index 049d4352ae3..c5975b1a35e 100644 --- a/erpnext/buying/doctype/purchase_order/purchase_order.py +++ b/erpnext/buying/doctype/purchase_order/purchase_order.py @@ -700,7 +700,7 @@ class PurchaseOrder(BuyingController): def update_subcontracting_order_status(self): from erpnext.subcontracting.doctype.subcontracting_order.subcontracting_order import ( - update_subcontracting_order_status as update_sco_status, + set_subcontracting_order_status as update_sco_status, ) if self.is_subcontracted and not self.is_old_subcontracting_flow: diff --git a/erpnext/controllers/subcontracting_inward_controller.py b/erpnext/controllers/subcontracting_inward_controller.py index 490f7204d2a..abc91afd5b8 100644 --- a/erpnext/controllers/subcontracting_inward_controller.py +++ b/erpnext/controllers/subcontracting_inward_controller.py @@ -1119,10 +1119,10 @@ class SubcontractingInwardController: def update_inward_order_status(self): if self.subcontracting_inward_order: from erpnext.subcontracting.doctype.subcontracting_inward_order.subcontracting_inward_order import ( - update_subcontracting_inward_order_status, + set_subcontracting_inward_order_status, ) - update_subcontracting_inward_order_status(self.subcontracting_inward_order) + set_subcontracting_inward_order_status(self.subcontracting_inward_order) @frappe.whitelist() diff --git a/erpnext/selling/doctype/sales_order/sales_order.py b/erpnext/selling/doctype/sales_order/sales_order.py index 48f2bbf5e6d..37cc541f411 100755 --- a/erpnext/selling/doctype/sales_order/sales_order.py +++ b/erpnext/selling/doctype/sales_order/sales_order.py @@ -614,7 +614,7 @@ class SalesOrder(SellingController): def update_subcontracting_order_status(self): from erpnext.subcontracting.doctype.subcontracting_inward_order.subcontracting_inward_order import ( - update_subcontracting_inward_order_status as update_scio_status, + set_subcontracting_inward_order_status as update_scio_status, ) if self.is_subcontracted: diff --git a/erpnext/stock/doctype/stock_entry/stock_entry.py b/erpnext/stock/doctype/stock_entry/stock_entry.py index 7784e9f98c0..ae536b8dc42 100644 --- a/erpnext/stock/doctype/stock_entry/stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/stock_entry.py @@ -3972,10 +3972,12 @@ class StockEntry(StockController, SubcontractingInwardController): def update_subcontracting_order_status(self): if self.subcontracting_order and self.purpose in ["Send to Subcontractor", "Material Transfer"]: from erpnext.subcontracting.doctype.subcontracting_order.subcontracting_order import ( - update_subcontracting_order_status, + set_subcontracting_order_status, ) - update_subcontracting_order_status(self.subcontracting_order) + # Trusted submit/cancel flow — a Stock operation must not require Subcontracting Order + # write permission, so use the no-check internal helper (not the whitelisted boundary). + set_subcontracting_order_status(self.subcontracting_order) def update_pick_list_status(self): from erpnext.stock.doctype.pick_list.pick_list import update_pick_list_status diff --git a/erpnext/subcontracting/doctype/subcontracting_inward_order/subcontracting_inward_order.py b/erpnext/subcontracting/doctype/subcontracting_inward_order/subcontracting_inward_order.py index 79f2ed33ed2..9687a070bda 100644 --- a/erpnext/subcontracting/doctype/subcontracting_inward_order/subcontracting_inward_order.py +++ b/erpnext/subcontracting/doctype/subcontracting_inward_order/subcontracting_inward_order.py @@ -550,10 +550,18 @@ class SubcontractingInwardOrder(SubcontractingController): return stock_entry.as_dict() +def set_subcontracting_inward_order_status(scio: str | Document, status: str | None = None): + if isinstance(scio, str): + scio = frappe.get_doc("Subcontracting Inward Order", scio) + + scio.update_status(status) + + @frappe.whitelist() def update_subcontracting_inward_order_status(scio: str | Document, status: str | None = None): + """Whitelisted boundary for direct API/UI calls — enforces write permission, then delegates.""" if isinstance(scio, str): scio = frappe.get_doc("Subcontracting Inward Order", scio) scio.check_permission("write") - scio.update_status(status) + set_subcontracting_inward_order_status(scio, status) diff --git a/erpnext/subcontracting/doctype/subcontracting_order/subcontracting_order.py b/erpnext/subcontracting/doctype/subcontracting_order/subcontracting_order.py index 29233f68195..043391145d2 100644 --- a/erpnext/subcontracting/doctype/subcontracting_order/subcontracting_order.py +++ b/erpnext/subcontracting/doctype/subcontracting_order/subcontracting_order.py @@ -483,10 +483,18 @@ def get_mapped_subcontracting_receipt(source_name, target_doc=None, items=None): return target_doc +def set_subcontracting_order_status(sco: str | Document, status: str | None = None): + if isinstance(sco, str): + sco = frappe.get_doc("Subcontracting Order", sco) + + sco.update_status(status) + + @frappe.whitelist() def update_subcontracting_order_status(sco: str | Document, status: str | None = None): + """Whitelisted boundary for direct API/UI calls — enforces write permission, then delegates.""" if isinstance(sco, str): sco = frappe.get_doc("Subcontracting Order", sco) sco.check_permission("write") - sco.update_status(status) + set_subcontracting_order_status(sco, status) diff --git a/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py b/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py index 96d3b861cf0..bf803fc3d9a 100644 --- a/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py +++ b/erpnext/subcontracting/doctype/subcontracting_order/test_subcontracting_order.py @@ -336,6 +336,62 @@ class TestSubcontractingOrder(ERPNextTestSuite): bin_after_cancel_sco.reserved_qty_for_sub_contract, bin_before_sco.reserved_qty_for_sub_contract ) + def test_send_to_subcontractor_ste_submit_without_sco_write_permission(self): + """A Stock-only user (can submit Stock Entries but has no Subcontracting Order write) must be + able to submit and cancel a 'Send to Subcontractor' Stock Entry. The SCO status update on the + on_submit/on_cancel path goes through the no-permission-check internal helper, not the + whitelisted API boundary. + + Regression: the permission hardening put check_permission('write') on the shared status + function, so a Stock Manager (no SCO write) hit PermissionError submitting/cancelling the + Stock Entry. The suite otherwise runs as Administrator and never caught it.""" + from frappe.core.doctype.user_permission.test_user_permission import create_user + + make_stock_entry(target="_Test Warehouse - _TC", item_code="_Test Item", qty=10, basic_rate=100) + + service_items = [ + { + "warehouse": "_Test Warehouse - _TC", + "item_code": "Subcontracted Service Item 1", + "qty": 10, + "rate": 100, + "fg_item": "_Test FG Item", + "fg_item_qty": 10, + }, + ] + sco = get_subcontracting_order(service_items=service_items) + + rm_items = [ + { + "item_code": "_Test FG Item", + "rm_item_code": "_Test Item", + "item_name": "_Test Item", + "qty": 10, + "warehouse": "_Test Warehouse - _TC", + "rate": 100, + "amount": 1000, + "stock_uom": "Nos", + }, + ] + ste = frappe.get_doc(make_rm_stock_entry(sco.name, rm_items)) + ste.to_warehouse = "_Test Warehouse 1 - _TC" + ste.save() + + stock_user = create_user("test_sco_stock_only@example.com", "Stock Manager") + self.assertFalse( + frappe.has_permission("Subcontracting Order", "write", user=stock_user.name), + "Precondition: the Stock-only user must not have Subcontracting Order write permission.", + ) + + frappe.set_user(stock_user.name) + try: + ste.reload() + ste.submit() # must not raise PermissionError on the SCO status update + ste.reload() + ste.cancel() # same on the cancel path + finally: + frappe.set_user("Administrator") + def test_exploded_items(self): item_code = "_Test Subcontracted FG Item 11" make_subcontracted_item(item_code=item_code) From ede13cb3bdd4aa6ec16fecdf49cd107617c5616e Mon Sep 17 00:00:00 2001 From: Rohit Waghchaure Date: Sun, 14 Jun 2026 23:50:23 +0530 Subject: [PATCH 2/2] refactor: consolidate duplicate get_party_bank_account into bank_account.py --- erpnext/accounts/doctype/payment_request/payment_request.py | 3 ++- erpnext/accounts/party.py | 6 ------ 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/erpnext/accounts/doctype/payment_request/payment_request.py b/erpnext/accounts/doctype/payment_request/payment_request.py index e16e132957f..d2dcb2ea795 100644 --- a/erpnext/accounts/doctype/payment_request/payment_request.py +++ b/erpnext/accounts/doctype/payment_request/payment_request.py @@ -11,11 +11,12 @@ from erpnext import get_company_currency from erpnext.accounts.doctype.accounting_dimension.accounting_dimension import ( get_accounting_dimensions, ) +from erpnext.accounts.doctype.bank_account.bank_account import get_party_bank_account from erpnext.accounts.doctype.payment_entry.payment_entry import ( get_payment_entry, ) from erpnext.accounts.doctype.subscription_plan.subscription_plan import get_plan_rate -from erpnext.accounts.party import get_party_account, get_party_bank_account +from erpnext.accounts.party import get_party_account from erpnext.accounts.utils import get_account_currency, get_advance_payment_doctypes, get_currency_precision from erpnext.utilities import payment_app_import_guard diff --git a/erpnext/accounts/party.py b/erpnext/accounts/party.py index 15645602965..7f0583f0d1d 100644 --- a/erpnext/accounts/party.py +++ b/erpnext/accounts/party.py @@ -507,12 +507,6 @@ def get_party_advance_account(party_type, party, company): return account -def get_party_bank_account(party_type: str, party: str): - return frappe.db.get_value( - "Bank Account", {"party_type": party_type, "party": party, "is_default": 1, "disabled": 0}, "name" - ) - - def get_party_account_currency(party_type, party, company): def generator(): party_account = get_party_account(party_type, party, company)