fix: Show server script name in traceback (#23676)

* fix: Show server script name in traceback

* chore: typo

Co-authored-by: Sagar Vora <sagar@resilient.tech>

---------

Co-authored-by: Sagar Vora <sagar@resilient.tech>

frappe/core/doctype/report/report.py

@@ -183,7 +183,7 @@ def execute_module(self, filters):
 	def execute_script(self, filters):
 		# server script
 		loc = {"filters": frappe._dict(filters), "data": None, "result": None}
-		safe_exec(self.report_script, None, loc)
+		safe_exec(self.report_script, None, loc, script_filename=f"Report {self.name}")
 		if loc["data"]:
 			return loc["data"]
 		else:

frappe/core/doctype/server_script/server_script.py

@@ -155,7 +155,12 @@ def execute_doc(self, doc: Document):
 		Args:
 		        doc (Document): Executes script with for a certain document's events
 		"""
-		safe_exec(self.script, _locals={"doc": doc}, restrict_commit_rollback=True)
+		safe_exec(
+			self.script,
+			_locals={"doc": doc},
+			restrict_commit_rollback=True,
+			script_filename=self.name,
+		)
 
 	def execute_scheduled_method(self):
 		"""Specific to Scheduled Jobs via Server Scripts
@@ -166,7 +171,7 @@ def execute_scheduled_method(self):
 		if self.script_type != "Scheduler Event":
 			raise frappe.DoesNotExistError
 
-		safe_exec(self.script)
+		safe_exec(self.script, script_filename=self.name)
 
 	def get_permission_query_conditions(self, user: str) -> list[str]:
 		"""Specific to Permission Query Server Scripts
@@ -178,7 +183,7 @@ def get_permission_query_conditions(self, user: str) -> list[str]:
 		        list: Returns list of conditions defined by rules in self.script
 		"""
 		locals = {"user": user, "conditions": ""}
-		safe_exec(self.script, None, locals)
+		safe_exec(self.script, None, locals, script_filename=self.name)
 		if locals["conditions"]:
 			return locals["conditions"]
 
@@ -278,7 +283,7 @@ def execute_api_server_script(script=None, *args, **kwargs):
 		raise frappe.PermissionError
 
 	# output can be stored in flags
-	_globals, _locals = safe_exec(script.script)
+	_globals, _locals = safe_exec(script.script, script_filename=script.name)
 
 	return _globals.frappe.flags
 

frappe/desk/doctype/system_console/system_console.py

@@ -28,7 +28,7 @@ def run(self):
 		try:
 			frappe.local.debug_log = []
 			if self.type == "Python":
-				safe_exec(self.console)
+				safe_exec(self.console, script_filename="System Console")
 				self.output = "\n".join(frappe.debug_log)
 			elif self.type == "SQL":
 				self.output = frappe.as_json(read_sql(self.console, as_dict=1))

frappe/query_builder/utils.py

@@ -107,28 +107,29 @@ def execute_child_queries(queries, result):
 	def prepare_query(query):
 		import inspect
 
+		from frappe.utils.safe_exec import SERVER_SCRIPT_FILE_PREFIX
+
 		param_collector = NamedParameterWrapper()
 		query = query.get_sql(param_wrapper=param_collector)
 		if frappe.flags.in_safe_exec:
 			from frappe.utils.safe_exec import check_safe_sql_query
 
 			if not check_safe_sql_query(query, throw=False):
 				callstack = inspect.stack()
-				if len(callstack) >= 3 and ".py" in callstack[2].filename:
-					# ignore any query builder methods called from python files
-					# assumption is that those functions are whitelisted already.
-
-					# since query objects are patched everywhere any query.run()
-					# will have callstack like this:
-					# frame0: this function prepare_query()
-					# frame1: execute_query()
-					# frame2: frame that called `query.run()`
-					#
-					# if frame2 is server script <serverscript> is set as the filename
-					# it shouldn't be allowed.
-					pass
-				else:
+
+				# This check is required because QB can execute from anywhere and we can not
+				# reliably provide a safe version for it in server scripts.
+
+				# since query objects are patched everywhere any query.run()
+				# will have callstack like this:
+				# frame0: this function prepare_query()
+				# frame1: execute_query()
+				# frame2: frame that called `query.run()`
+				#
+				# if frame2 is server script <serverscript> is set as the filename it shouldn't be allowed.
+				if len(callstack) >= 3 and SERVER_SCRIPT_FILE_PREFIX in callstack[2].filename:
 					raise frappe.PermissionError("Only SELECT SQL allowed in scripting")
+
 		return query, param_collector.get_parameters()
 
 	builder_class = frappe.qb._BuilderClasss

frappe/recorder.py

@@ -41,11 +41,13 @@ def sql(*args, **kwargs):
 
 
 def get_current_stack_frames():
+	from frappe.utils.safe_exec import SERVER_SCRIPT_FILE_PREFIX
+
 	try:
 		current = inspect.currentframe()
 		frames = inspect.getouterframes(current, context=10)
 		for frame, filename, lineno, function, context, index in list(reversed(frames))[:-2]:
-			if "/apps/" in filename or "<serverscript>" in filename:
+			if "/apps/" in filename or SERVER_SCRIPT_FILE_PREFIX in filename:
 				yield {
 					"filename": TRACEBACK_PATH_PATTERN.sub("", filename),
 					"lineno": lineno,

frappe/utils/error.py

@@ -146,19 +146,20 @@ def guess_exception_source(exception: str) -> str | None:
 
 	- For unhandled exception last python file from apps folder is responsible.
 	- For frappe.throws the exception source is possibly present after skipping frappe.throw frames
-	- For server script the file name is `<serverscript>`
+	- For server script the file name contains SERVER_SCRIPT_FILE_PREFIX
 
 	"""
+	from frappe.utils.safe_exec import SERVER_SCRIPT_FILE_PREFIX
+
 	with suppress(Exception):
 		installed_apps = frappe.get_installed_apps()
 		app_priority = {app: installed_apps.index(app) for app in installed_apps}
 
 		APP_NAME_REGEX = re.compile(r".*File.*apps/(?P<app_name>\w+)/\1/")
-		SERVER_SCRIPT_FRAME = re.compile(r".*<serverscript>")
 
 		apps = Counter()
 		for line in reversed(exception.splitlines()):
-			if SERVER_SCRIPT_FRAME.match(line):
+			if SERVER_SCRIPT_FILE_PREFIX in line:
 				return "Server Script"
 
 			if matches := APP_NAME_REGEX.match(line):

frappe/utils/safe_exec.py

@@ -36,6 +36,7 @@ class ServerScriptNotEnabled(frappe.PermissionError):
 ARGUMENT_NOT_SET = object()
 
 SAFE_EXEC_CONFIG_KEY = "server_script_enabled"
+SERVER_SCRIPT_FILE_PREFIX = "<serverscript>"
 
 
 class NamespaceDict(frappe._dict):
@@ -65,7 +66,14 @@ def is_safe_exec_enabled() -> bool:
 	return bool(frappe.get_common_site_config().get(SAFE_EXEC_CONFIG_KEY))
 
 
-def safe_exec(script, _globals=None, _locals=None, restrict_commit_rollback=False):
+def safe_exec(
+	script: str,
+	_globals: dict | None = None,
+	_locals: dict | None = None,
+	*,
+	restrict_commit_rollback: bool = False,
+	script_filename: str | None = None,
+):
 	if not is_safe_exec_enabled():
 
 		msg = _("Server Scripts are disabled. Please enable server scripts from bench configuration.")
@@ -84,10 +92,14 @@ def safe_exec(script, _globals=None, _locals=None, restrict_commit_rollback=Fals
 		exec_globals.frappe.db.pop("rollback", None)
 		exec_globals.frappe.db.pop("add_index", None)
 
+	filename = SERVER_SCRIPT_FILE_PREFIX
+	if script_filename:
+		filename += f": {frappe.scrub(script_filename)}"
+
 	with safe_exec_flags(), patched_qb():
 		# execute script compiled by RestrictedPython
 		exec(
-			compile_restricted(script, filename="<serverscript>", policy=FrappeTransformer),
+			compile_restricted(script, filename=filename, policy=FrappeTransformer),
 			exec_globals,
 			_locals,
 		)

frappe/website/doctype/web_page/web_page.py

@@ -78,7 +78,7 @@ def get_context(self, context):
 
 		if self.context_script:
 			_locals = dict(context=frappe._dict())
-			safe_exec(self.context_script, None, _locals)
+			safe_exec(self.context_script, None, _locals, script_filename=f"web page {self.name}")
 			context.update(_locals["context"])
 
 		self.render_dynamic(context)