diff --git a/Code-Security-Guidelines.md b/Code-Security-Guidelines.md index 493956c..01e57e6 100644 --- a/Code-Security-Guidelines.md +++ b/Code-Security-Guidelines.md @@ -67,7 +67,7 @@ Let's say you have created an API method `create_document`: **api.py** ```py @frappe.whitelist() -def create_document(values): +def create_document(values: dict): doc = frappe.get_doc(values).insert(ignore_permissions=True) return doc ``` @@ -78,7 +78,7 @@ You can use a combination of `frappe.only_for` method to restrict the method to ```py @frappe.whitelist() -def create_document(values): +def create_document(values: dict): frappe.only_for('System User') if values['doctype'] not in ('ToDo', 'Note', 'Task'): @@ -100,7 +100,7 @@ Example: ```python @frappe.whitelist() -def exec_method(method, *args, **kwargs): +def exec_method(method: str, *args, **kwargs): return frappe.get_attr(method)(*args, **kwargs) # You just allowed user to call anything in python environment ``` @@ -119,7 +119,7 @@ Example: ```python @frappe.whitelist() -def get_file(path): +def get_file(path: str): return open(path).read() # This allows reading everything on server. ``` @@ -136,8 +136,35 @@ Example: ```diff @frappe.whitelist() - def better_get_doc(doctype, name): + def better_get_doc(doctype: str, name: str): doc = frappe.get_doc(doctype, name) # This allows bypassing all permission and reading every document in system + doc.check_permission("read") # this makes sure logged in user has correct permission to read the document return doc -``` \ No newline at end of file +``` + +## Check parameter types + +Always check if the parameters passed to your whitelisted method have the type you expect. For example, if you accept a filter value for a specific company, like `"Example Corp"` users could instead pass a different filter object like `["is", "set"]`, thus changing the behavior of your code. + +In v15+, type annotations will be checked automatically, you just need to provide them: + +```diff + @frappe.whitelist() +- def get_user(name): ++ def get_user(name: str): + doc = frappe.get_doc("User", name) + doc.check_permission("read") + return doc +``` + +In v14 and below, you need to check types explicitly: + +```diff + @frappe.whitelist() + def get_user(name): ++ if not isinstance(name, str): ++ raise ValueError("name should be a string") + doc = frappe.get_doc("User", name) + doc.check_permission("read") + return doc +```