From 63afc0601b4bdd56face8a615c39d330c91fe111 Mon Sep 17 00:00:00 2001
From: Faris Ansari <netchamp.faris@gmail.com>
Date: Fri, 2 May 2025 19:08:00 +0530
Subject: [PATCH] fix: restrict child table access if user has only "select" on
 parent

---
 frappe/database/query.py   | 16 +++++++++---
 frappe/tests/test_query.py | 51 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/frappe/database/query.py b/frappe/database/query.py
index ec2fbe6ee3..47a8b4d887 100644
--- a/frappe/database/query.py
+++ b/frappe/database/query.py
@@ -391,8 +391,8 @@ class Engine:
 		if not fields:
 			return []
 
-		# Handle direct pypika Field or Function objects
-		if isinstance(fields, Field | AggregateFunction):
+		# return if fields is already a pypika Term
+		if isinstance(fields, Term):
 			return [fields]
 
 		initial_field_list = []
@@ -545,17 +545,23 @@ class Engine:
 	def apply_field_permissions(self):
 		"""Filter the list of fields based on permlevel."""
 		allowed_fields = []
+		parent_permission_type = self.get_permission_type(self.doctype)
+
 		permitted_fields_set = set(
 			get_permitted_fields(
 				doctype=self.doctype,
 				parenttype=self.parent_doctype,
-				permission_type=self.get_permission_type(self.doctype),
+				permission_type=parent_permission_type,
 				ignore_virtual=True,
 			)
 		)
 
 		for field in self.fields:
 			if isinstance(field, ChildTableField):
+				if parent_permission_type == "select":
+					# Skip child table fields if parent permission is only 'select'
+					continue
+
 				# Cache permitted fields for child doctypes if accessed multiple times
 				permitted_child_fields_set = set(
 					get_permitted_fields(
@@ -573,6 +579,10 @@ class Engine:
 				if field.link_fieldname in permitted_fields_set:
 					allowed_fields.append(field)
 			elif isinstance(field, ChildQuery):
+				if parent_permission_type == "select":
+					# Skip child queries if parent permission is only 'select'
+					continue
+
 				# Cache permitted fields for the child doctype of the query
 				permitted_child_fields_set = set(
 					get_permitted_fields(
diff --git a/frappe/tests/test_query.py b/frappe/tests/test_query.py
index d6991f3358..988af95aaa 100644
--- a/frappe/tests/test_query.py
+++ b/frappe/tests/test_query.py
@@ -2,6 +2,7 @@ import itertools
 
 import frappe
 from frappe.core.doctype.doctype.test_doctype import new_doctype
+from frappe.permissions import add_permission, update_permission_property
 from frappe.query_builder import Field
 from frappe.query_builder.functions import Abs, Count, Ifnull, Max, Now, Timestamp
 from frappe.tests import IntegrationTestCase
@@ -734,6 +735,56 @@ class TestQuery(IntegrationTestCase):
 
 			test_post.delete()
 
+	def test_child_table_access_with_select_permission(self):
+		"""Test that child table fields are inaccessible if user only has select perm on parent."""
+
+		test_role = "Select Note Test Role"
+		test_user_email = "test2@example.com"  # Use existing test user
+		test_note_title = "Child Select Test Note"
+
+		# Cleanup
+		frappe.set_user("Administrator")
+		test_user = frappe.get_doc("User", test_user_email)
+		test_user.remove_roles(test_role)
+		frappe.delete_doc("Role", test_role, ignore_missing=True, force=True)
+		frappe.delete_doc("Note", {"title": test_note_title}, ignore_missing=True, force=True)
+
+		# Setup Role with 'select' on Note and 'read' on Note Seen By
+		frappe.get_doc({"doctype": "Role", "role_name": test_role}).insert(ignore_if_duplicate=True)
+		# Grant select on Note, read on Note Seen By
+		add_permission("Note", test_role, 0, ptype="select")
+		add_permission("Note Seen By", test_role, 0, ptype="read")
+		# Ensure no read permission on Note for this role by explicitly setting it to 0
+		update_permission_property("Note", test_role, 0, "read", 0, validate=False)
+		test_user.add_roles(test_role)
+
+		note = frappe.get_doc(
+			doctype="Note", title=test_note_title, public=1, seen_by=[{"user": "Administrator"}]
+		).insert(ignore_permissions=True)
+
+		frappe.set_user(test_user_email)
+		query = frappe.qb.get_query(
+			"Note",
+			filters={"name": note.name},
+			fields=["name", {"seen_by": ["user"]}],
+			ignore_permissions=False,
+		)
+		result = query.run(as_dict=True)
+
+		self.assertEqual(len(result), 1, "Should find the note record")
+		self.assertIn("name", result[0], "Parent field 'name' should be accessible")
+		self.assertNotIn(
+			"seen_by",
+			result[0],
+			"Child table field 'seen_by' should NOT be accessible with only 'select' on parent",
+		)
+
+		# Cleanup
+		frappe.set_user("Administrator")
+		note.delete(ignore_permissions=True)
+		test_user.remove_roles(test_role)
+		frappe.delete_doc("Role", test_role, force=True, ignore_permissions=True)
+
 	def test_nested_permission(self):
 		"""Test permission on nested doctypes"""
 		frappe.set_user("Administrator")