From 8d3894e8a537540899c0412e2a48724014b6c304 Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Fri, 27 Nov 2020 11:01:49 +0530
Subject: [PATCH 1/4] fix: Validate Python syntax on saves

---
 frappe/core/doctype/server_script/server_script.py | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/frappe/core/doctype/server_script/server_script.py b/frappe/core/doctype/server_script/server_script.py
index 839b784651..ded397d5e3 100644
--- a/frappe/core/doctype/server_script/server_script.py
+++ b/frappe/core/doctype/server_script/server_script.py
@@ -4,6 +4,8 @@
 
 from __future__ import unicode_literals
 
+import ast
+
 import frappe
 from frappe.model.document import Document
 from frappe.utils.safe_exec import safe_exec
@@ -11,9 +13,9 @@ from frappe import _
 
 
 class ServerScript(Document):
-	@staticmethod
-	def validate():
+	def validate(self):
 		frappe.only_for('Script Manager', True)
+		ast.parse(self.script)
 
 	@staticmethod
 	def on_update():

From 5babacac3eac0e4dd2c7734a779b59641b96c94b Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Fri, 27 Nov 2020 11:02:37 +0530
Subject: [PATCH 2/4] fix: Show function not available in namespace instead of
 nothing

Prior to this, frappe._dict was being used to inject functions to the
server script namespaces. This meant unimplemented methods
returned None and we'd get a NoneType not callable error
---
 frappe/utils/safe_exec.py | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py
index fee6b404ac..50893330be 100644
--- a/frappe/utils/safe_exec.py
+++ b/frappe/utils/safe_exec.py
@@ -13,7 +13,17 @@ from frappe.www.printview import get_visible_columns
 import frappe.exceptions
 import frappe.integrations.utils
 
-class ServerScriptNotEnabled(frappe.PermissionError): pass
+class ServerScriptNotEnabled(frappe.PermissionError):
+	pass
+
+class NamespaceDict(frappe._dict):
+	"""Raise AttributeError if function not found in namespace"""
+	def __getattr__(self, key):
+		ret = self.get(key)
+		if (not ret and key.startswith("__")) or (key not in self):
+			raise AttributeError(f"module has no attribute '{key}'")
+		return ret
+
 
 def safe_exec(script, _globals=None, _locals=None):
 	# script reports must be enabled via site_config.json
@@ -46,13 +56,13 @@ def get_safe_globals():
 
 	user = getattr(frappe.local, "session", None) and frappe.local.session.user or "Guest"
 
-	out = frappe._dict(
+	out = NamespaceDict(
 		# make available limited methods of frappe
 		json=json,
 		dict=dict,
 		log=frappe.log,
 		_dict=frappe._dict,
-		frappe=frappe._dict(
+		frappe=NamespaceDict(
 			flags=frappe._dict(),
 			format=frappe.format_value,
 			format_value=frappe.format_value,
@@ -112,7 +122,7 @@ def get_safe_globals():
 		out.get_visible_columns = get_visible_columns
 		out.frappe.date_format = date_format
 		out.frappe.time_format = time_format
-		out.frappe.db = frappe._dict(
+		out.frappe.db = NamespaceDict(
 			get_list = frappe.get_list,
 			get_all = frappe.get_all,
 			get_value = frappe.db.get_value,

From 4810d07a8a4823a31ceef66b0896a991123ece6e Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Mon, 30 Nov 2020 15:23:12 +0530
Subject: [PATCH 3/4] test: Add tests for throwing AttributeError if method not
 found

---
 .../doctype/server_script/test_server_script.py    | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/frappe/core/doctype/server_script/test_server_script.py b/frappe/core/doctype/server_script/test_server_script.py
index 3356e584af..256cea57d7 100644
--- a/frappe/core/doctype/server_script/test_server_script.py
+++ b/frappe/core/doctype/server_script/test_server_script.py
@@ -45,6 +45,15 @@ frappe.response['message'] = 'hello'
 		allow_guest = 1,
 		script = '''
 frappe.flags = 'hello'
+'''
+	),
+	dict(
+		name='test_invalid_namespace_method',
+		script_type = 'DocType Event',
+		doctype_event = 'Before Insert',
+		reference_doctype = 'Note',
+		script = '''
+frappe.method_that_doesnt_exist("do some magic")
 '''
 	)
 ]
@@ -85,3 +94,8 @@ class TestServerScript(unittest.TestCase):
 
 	def test_api_return(self):
 		self.assertEqual(frappe.get_doc('Server Script', 'test_return_value').execute_method(), 'hello')
+
+	def test_attribute_error(self):
+		"""Raise AttributeError if method not found in Namespace"""
+		note = frappe.get_doc({"doctype": "Note", "title": "Test Note: Server Script"})
+		self.assertRaises(AttributeError, note.insert)

From d5d0bc8ea9983438366c059edea0be19a26bf359 Mon Sep 17 00:00:00 2001
From: Gavin D'souza <gavin18d@gmail.com>
Date: Mon, 30 Nov 2020 15:23:42 +0530
Subject: [PATCH 4/4] fix: Return dummy function to avoid NoneType not callable

---
 frappe/utils/safe_exec.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/frappe/utils/safe_exec.py b/frappe/utils/safe_exec.py
index 50893330be..2aacf5eda8 100644
--- a/frappe/utils/safe_exec.py
+++ b/frappe/utils/safe_exec.py
@@ -21,7 +21,9 @@ class NamespaceDict(frappe._dict):
 	def __getattr__(self, key):
 		ret = self.get(key)
 		if (not ret and key.startswith("__")) or (key not in self):
-			raise AttributeError(f"module has no attribute '{key}'")
+			def default_function(*args, **kwargs):
+				raise AttributeError(f"module has no attribute '{key}'")
+			return default_function
 		return ret