From 2095073688c63db15696c6f526e84e569039858e Mon Sep 17 00:00:00 2001 From: Mihir Kandoi Date: Tue, 28 Jul 2026 20:15:19 +0530 Subject: [PATCH] fix: check company permission before reading sample retention stock validate_sample_quantity and move_sample_to_retention_warehouse are whitelisted and take company from the caller, which selects whose retention warehouse gets read. The retained batch qty then reaches the return value and the max-retained warning, so an authenticated user could probe another company's stock with a known item and batch. Gate the shared company -> warehouse resolution on read permission for the Company, which respects User Permissions. validate_sample_quantity only grew a company argument in this branch; move_sample_to_retention_warehouse already took one, so this closes that path too. --- .../stock_entry/services/manufacturing.py | 3 ++ .../doctype/stock_entry/test_stock_entry.py | 34 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/erpnext/stock/doctype/stock_entry/services/manufacturing.py b/erpnext/stock/doctype/stock_entry/services/manufacturing.py index 4d450d4764d..26655f41355 100644 --- a/erpnext/stock/doctype/stock_entry/services/manufacturing.py +++ b/erpnext/stock/doctype/stock_entry/services/manufacturing.py @@ -1286,6 +1286,9 @@ def validate_sample_quantity( def get_sample_retention_warehouse(company: str) -> str: + # `company` arrives from whitelisted callers, so it decides which company's stock gets read. + frappe.has_permission("Company", "read", company, throw=True) + warehouse = frappe.get_cached_value("Company", company, "sample_retention_warehouse") if not warehouse: frappe.throw( diff --git a/erpnext/stock/doctype/stock_entry/test_stock_entry.py b/erpnext/stock/doctype/stock_entry/test_stock_entry.py index c4e9cb3dfae..74a11dd41ae 100644 --- a/erpnext/stock/doctype/stock_entry/test_stock_entry.py +++ b/erpnext/stock/doctype/stock_entry/test_stock_entry.py @@ -3260,6 +3260,40 @@ class TestStockEntryCoverage(ERPNextTestSuite): "_Sample Batch", ) + def test_sample_retention_warehouse_denied_for_other_company(self): + """`company` comes from whitelisted callers, so it must not read another company's stock.""" + from erpnext.stock.doctype.stock_entry.services.manufacturing import ( + get_sample_retention_warehouse, + ) + + frappe.db.set_value( + "Company", "_Test Company", "sample_retention_warehouse", "_Test Warehouse 1 - _TC" + ) + + user = "test_sample_retention_perm@example.com" + if not frappe.db.exists("User", user): + frappe.get_doc( + { + "doctype": "User", + "email": user, + "first_name": "Sample Retention", + "send_welcome_email": 0, + "roles": [{"role": "Stock User"}], + } + ).insert(ignore_permissions=True) + + frappe.get_doc( + { + "doctype": "User Permission", + "user": user, + "allow": "Company", + "for_value": "_Test Company 1", + } + ).insert(ignore_permissions=True) + + with self.set_user(user): + self.assertRaises(frappe.PermissionError, get_sample_retention_warehouse, "_Test Company") + # ── get_expired_batches ──────────────────────────────────────────────────── def test_get_expired_batches_includes_expired_batch(self):