From 1ed6e10a8a5184e010b7cfd621f567604b3a13d1 Mon Sep 17 00:00:00 2001 From: Jatin3128 Date: Fri, 31 Jul 2026 12:40:08 +0530 Subject: [PATCH] fix: permission-check the source row before returning its rate The rate lock reads the linked source row with a direct db.get_value, which bypasses permissions on a whitelisted endpoint. Only return the source pricing when the caller can read the source document, so a crafted request cannot disclose another document's rate. Covered by a test. --- erpnext/stock/get_item_details.py | 9 +++- erpnext/stock/tests/test_get_item_details.py | 46 ++++++++++++++++++++ 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/erpnext/stock/get_item_details.py b/erpnext/stock/get_item_details.py index b178abf5558..8dc0bd0cb3b 100644 --- a/erpnext/stock/get_item_details.py +++ b/erpnext/stock/get_item_details.py @@ -234,7 +234,14 @@ def get_rate_locked_source_row(ctx: ItemDetailsCtx, doc) -> frappe._dict | None: for link_field, source_doctype in source_fields.items(): if source_name := row.get(link_field): - return frappe.db.get_value(source_doctype, source_name, LOCKED_RATE_FIELDS, as_dict=True) + # a direct read would bypass permissions; only return source pricing to a + # caller allowed to read the source document + source = frappe.db.get_value( + source_doctype, source_name, [*LOCKED_RATE_FIELDS, "parent", "parenttype"], as_dict=True + ) + if source and frappe.has_permission(source.parenttype, doc=source.parent): + return source + return None return None diff --git a/erpnext/stock/tests/test_get_item_details.py b/erpnext/stock/tests/test_get_item_details.py index 12159082dec..79fd6fe83f1 100644 --- a/erpnext/stock/tests/test_get_item_details.py +++ b/erpnext/stock/tests/test_get_item_details.py @@ -411,3 +411,49 @@ class TestGetItemDetail(ERPNextTestSuite): frappe.db.set_single_value("Buying Settings", "maintain_same_rate", original) frappe.db.set_single_value("Buying Settings", "maintain_same_rate_action", original_action) frappe.clear_cache(doctype="Buying Settings") + + def test_rate_lock_source_lookup_checks_permission(self): + """The lock reads source pricing via a direct DB read, so it must not disclose a + source document's pricing to a caller who cannot read that document. + """ + from erpnext.buying.doctype.purchase_order.test_purchase_order import create_purchase_order + from erpnext.stock.get_item_details import get_rate_locked_source_row + + original = frappe.db.get_single_value("Buying Settings", "maintain_same_rate") + frappe.db.set_single_value("Buying Settings", "maintain_same_rate", 1) + frappe.clear_cache(doctype="Buying Settings") + + role, email = "_Test Role Without PO Access", "_test_rate_lock_probe@example.com" + try: + po = create_purchase_order(item_code="_Test Item", qty=1, rate=90) + pr_doc = { + "doctype": "Purchase Receipt", + "items": [{"name": "r1", "item_code": "_Test Item", "purchase_order_item": po.items[0].name}], + } + ctx = frappe._dict(doctype="Purchase Receipt", child_docname="r1") + + # an authorized caller receives the source row + self.assertIsNotNone(get_rate_locked_source_row(ctx.copy(), dict(pr_doc))) + + if not frappe.db.exists("Role", role): + frappe.get_doc({"doctype": "Role", "role_name": role, "desk_access": 1}).insert( + ignore_permissions=True + ) + if not frappe.db.exists("User", email): + frappe.get_doc( + { + "doctype": "User", + "email": email, + "first_name": "Probe", + "send_welcome_email": 0, + "roles": [{"role": role}], + } + ).insert(ignore_permissions=True) + + frappe.set_user(email) + # a caller who cannot read the Purchase Order gets nothing + self.assertIsNone(get_rate_locked_source_row(ctx.copy(), dict(pr_doc))) + finally: + frappe.set_user("Administrator") + frappe.db.set_single_value("Buying Settings", "maintain_same_rate", original) + frappe.clear_cache(doctype="Buying Settings")