ci: enable semgrep check on v13 branches and update rules (#25647)
* ci: enable semgrep on v13 branches
* ci: break semgrep steps for nicer output
* ci: update semgrep rules inline with frappe repo
diff --git a/.flake8 b/.flake8
index 399b176..56c9b9a 100644
--- a/.flake8
+++ b/.flake8
@@ -29,4 +29,5 @@
B950,
W191,
-max-line-length = 200
\ No newline at end of file
+max-line-length = 200
+exclude=.github/helper/semgrep_rules
diff --git a/.github/helper/semgrep_rules/frappe_correctness.py b/.github/helper/semgrep_rules/frappe_correctness.py
index 4798b92..745e646 100644
--- a/.github/helper/semgrep_rules/frappe_correctness.py
+++ b/.github/helper/semgrep_rules/frappe_correctness.py
@@ -4,25 +4,61 @@
from frappe.model.document import Document
+# ruleid: frappe-modifying-but-not-comitting
def on_submit(self):
if self.value_of_goods == 0:
frappe.throw(_('Value of goods cannot be 0'))
- # ruleid: frappe-modifying-after-submit
self.status = 'Submitted'
+
+# ok: frappe-modifying-but-not-comitting
def on_submit(self):
- if flt(self.per_billed) < 100:
- self.update_billing_status()
- else:
- # todook: frappe-modifying-after-submit
- self.status = "Completed"
- self.db_set("status", "Completed")
+ if self.value_of_goods == 0:
+ frappe.throw(_('Value of goods cannot be 0'))
+ self.status = 'Submitted'
+ self.db_set('status', 'Submitted')
-class TestDoc(Document):
- pass
+# ok: frappe-modifying-but-not-comitting
+def on_submit(self):
+ if self.value_of_goods == 0:
+ frappe.throw(_('Value of goods cannot be 0'))
+ x = "y"
+ self.status = x
+ self.db_set('status', x)
- def validate(self):
- #ruleid: frappe-modifying-child-tables-while-iterating
- for item in self.child_table:
- if item.value < 0:
- self.remove(item)
+
+# ok: frappe-modifying-but-not-comitting
+def on_submit(self):
+ x = "y"
+ self.status = x
+ self.save()
+
+# ruleid: frappe-modifying-but-not-comitting-other-method
+class DoctypeClass(Document):
+ def on_submit(self):
+ self.good_method()
+ self.tainted_method()
+
+ def tainted_method(self):
+ self.status = "uptate"
+
+
+# ok: frappe-modifying-but-not-comitting-other-method
+class DoctypeClass(Document):
+ def on_submit(self):
+ self.good_method()
+ self.tainted_method()
+
+ def tainted_method(self):
+ self.status = "update"
+ self.db_set("status", "update")
+
+# ok: frappe-modifying-but-not-comitting-other-method
+class DoctypeClass(Document):
+ def on_submit(self):
+ self.good_method()
+ self.tainted_method()
+ self.save()
+
+ def tainted_method(self):
+ self.status = "uptate"
diff --git a/.github/helper/semgrep_rules/frappe_correctness.yml b/.github/helper/semgrep_rules/frappe_correctness.yml
index 54df062..faab334 100644
--- a/.github/helper/semgrep_rules/frappe_correctness.yml
+++ b/.github/helper/semgrep_rules/frappe_correctness.yml
@@ -1,32 +1,93 @@
# This file specifies rules for correctness according to how frappe doctype data model works.
rules:
-- id: frappe-modifying-after-submit
+- id: frappe-modifying-but-not-comitting
patterns:
- - pattern: self.$ATTR = ...
- - pattern-inside: |
- def on_submit(self, ...):
+ - pattern: |
+ def $METHOD(self, ...):
...
+ self.$ATTR = ...
+ - pattern-not: |
+ def $METHOD(self, ...):
+ ...
+ self.$ATTR = ...
+ ...
+ self.db_set(..., self.$ATTR, ...)
+ - pattern-not: |
+ def $METHOD(self, ...):
+ ...
+ self.$ATTR = $SOME_VAR
+ ...
+ self.db_set(..., $SOME_VAR, ...)
+ - pattern-not: |
+ def $METHOD(self, ...):
+ ...
+ self.$ATTR = $SOME_VAR
+ ...
+ self.save()
- metavariable-regex:
metavariable: '$ATTR'
# this is negative look-ahead, add more attrs to ignore like (ignore|ignore_this_too|ignore_me)
- regex: '^(?!status_updater)(.*)$'
+ regex: '^(?!ignore_linked_doctypes|status_updater)(.*)$'
+ - metavariable-regex:
+ metavariable: "$METHOD"
+ regex: "(on_submit|on_cancel)"
message: |
- Doctype modified after submission. Please check if modification of self.$ATTR is commited to database.
+ DocType modified in self.$METHOD. Please check if modification of self.$ATTR is commited to database.
languages: [python]
severity: ERROR
-- id: frappe-modifying-after-cancel
+- id: frappe-modifying-but-not-comitting-other-method
patterns:
- - pattern: self.$ATTR = ...
- - pattern-inside: |
- def on_cancel(self, ...):
+ - pattern: |
+ class $DOCTYPE(...):
+ def $METHOD(self, ...):
...
- - metavariable-regex:
- metavariable: '$ATTR'
- regex: '^(?!ignore_linked_doctypes|status_updater)(.*)$'
+ self.$ANOTHER_METHOD()
+ ...
+
+ def $ANOTHER_METHOD(self, ...):
+ ...
+ self.$ATTR = ...
+ - pattern-not: |
+ class $DOCTYPE(...):
+ def $METHOD(self, ...):
+ ...
+ self.$ANOTHER_METHOD()
+ ...
+
+ def $ANOTHER_METHOD(self, ...):
+ ...
+ self.$ATTR = ...
+ ...
+ self.db_set(..., self.$ATTR, ...)
+ - pattern-not: |
+ class $DOCTYPE(...):
+ def $METHOD(self, ...):
+ ...
+ self.$ANOTHER_METHOD()
+ ...
+
+ def $ANOTHER_METHOD(self, ...):
+ ...
+ self.$ATTR = $SOME_VAR
+ ...
+ self.db_set(..., $SOME_VAR, ...)
+ - pattern-not: |
+ class $DOCTYPE(...):
+ def $METHOD(self, ...):
+ ...
+ self.$ANOTHER_METHOD()
+ ...
+ self.save()
+ def $ANOTHER_METHOD(self, ...):
+ ...
+ self.$ATTR = ...
+ - metavariable-regex:
+ metavariable: "$METHOD"
+ regex: "(on_submit|on_cancel)"
message: |
- Doctype modified after cancellation. Please check if modification of self.$ATTR is commited to database.
+ self.$ANOTHER_METHOD is called from self.$METHOD, check if changes to self.$ATTR are commited to database.
languages: [python]
severity: ERROR
diff --git a/.github/helper/semgrep_rules/translate.js b/.github/helper/semgrep_rules/translate.js
index 7b92fe2..9cdfb75 100644
--- a/.github/helper/semgrep_rules/translate.js
+++ b/.github/helper/semgrep_rules/translate.js
@@ -35,3 +35,10 @@
// ruleid: frappe-translation-js-splitting
__('You have {0} subscribers' +
'in your mailing list', [subscribers.length])
+
+// ok: frappe-translation-js-splitting
+__("Ctrl+Enter to add comment")
+
+// ruleid: frappe-translation-js-splitting
+__('You have {0} subscribers \
+ in your mailing list', [subscribers.length])
diff --git a/.github/helper/semgrep_rules/translate.yml b/.github/helper/semgrep_rules/translate.yml
index 3737da5..fa4ec9e 100644
--- a/.github/helper/semgrep_rules/translate.yml
+++ b/.github/helper/semgrep_rules/translate.yml
@@ -42,9 +42,10 @@
- id: frappe-translation-python-splitting
pattern-either:
- - pattern: _(...) + ... + _(...)
+ - pattern: _(...) + _(...)
- pattern: _("..." + "...")
- - pattern-regex: '_\([^\)]*\\\s*'
+ - pattern-regex: '_\([^\)]*\\\s*' # lines broken by `\`
+ - pattern-regex: '_\(\s*\n' # line breaks allowed by python for using ( )
message: |
Do not split strings inside translate function. Do not concatenate using translate functions.
Please refer: https://frappeframework.com/docs/user/en/translations
@@ -53,8 +54,8 @@
- id: frappe-translation-js-splitting
pattern-either:
- - pattern-regex: '__\([^\)]*[\+\\]\s*'
- - pattern: __('...' + '...')
+ - pattern-regex: '__\([^\)]*[\\]\s+'
+ - pattern: __('...' + '...', ...)
- pattern: __('...') + __('...')
message: |
Do not split strings inside translate function. Do not concatenate using translate functions.
diff --git a/.github/workflows/semgrep.yml b/.github/workflows/semgrep.yml
index df08263..389524e 100644
--- a/.github/workflows/semgrep.yml
+++ b/.github/workflows/semgrep.yml
@@ -4,6 +4,8 @@
pull_request:
branches:
- develop
+ - version-13-hotfix
+ - version-13-pre-release
jobs:
semgrep:
name: Frappe Linter
@@ -14,11 +16,19 @@
uses: actions/setup-python@v2
with:
python-version: 3.8
- - name: Run semgrep
+
+ - name: Setup semgrep
run: |
python -m pip install -q semgrep
git fetch origin $GITHUB_BASE_REF:$GITHUB_BASE_REF -q
+
+ - name: Semgrep errors
+ run: |
files=$(git diff --name-only --diff-filter=d $GITHUB_BASE_REF)
[[ -d .github/helper/semgrep_rules ]] && semgrep --severity ERROR --config=.github/helper/semgrep_rules --quiet --error $files
semgrep --config="r/python.lang.correctness" --quiet --error $files
+
+ - name: Semgrep warnings
+ run: |
+ files=$(git diff --name-only --diff-filter=d $GITHUB_BASE_REF)
[[ -d .github/helper/semgrep_rules ]] && semgrep --severity WARNING --severity INFO --config=.github/helper/semgrep_rules --quiet $files