Skip to content

Commit

Permalink
Merge pull request #25820 from deepeshgarg007/update-staging
Browse files Browse the repository at this point in the history
chore: Update branch
  • Loading branch information
deepeshgarg007 committed May 25, 2021
2 parents b777937 + 8f057b4 commit 4367f1c
Show file tree
Hide file tree
Showing 489 changed files with 12,137 additions and 5,847 deletions.
46 changes: 46 additions & 0 deletions .github/helper/install.sh
@@ -0,0 +1,46 @@
#!/bin/bash

set -e

cd ~ || exit

sudo apt-get install redis-server

sudo apt install nodejs

sudo apt install npm

pip install frappe-bench

git clone https://github.com/frappe/frappe --branch "${GITHUB_BASE_REF:-${GITHUB_REF##*/}}" --depth 1
bench init --skip-assets --frappe-path ~/frappe --python "$(which python)" frappe-bench

mkdir ~/frappe-bench/sites/test_site
cp -r "${GITHUB_WORKSPACE}/.github/helper/site_config.json" ~/frappe-bench/sites/test_site/

mysql --host 127.0.0.1 --port 3306 -u root -e "SET GLOBAL character_set_server = 'utf8mb4'"
mysql --host 127.0.0.1 --port 3306 -u root -e "SET GLOBAL collation_server = 'utf8mb4_unicode_ci'"

mysql --host 127.0.0.1 --port 3306 -u root -e "CREATE USER 'test_frappe'@'localhost' IDENTIFIED BY 'test_frappe'"
mysql --host 127.0.0.1 --port 3306 -u root -e "CREATE DATABASE test_frappe"
mysql --host 127.0.0.1 --port 3306 -u root -e "GRANT ALL PRIVILEGES ON \`test_frappe\`.* TO 'test_frappe'@'localhost'"

mysql --host 127.0.0.1 --port 3306 -u root -e "UPDATE mysql.user SET Password=PASSWORD('travis') WHERE User='root'"
mysql --host 127.0.0.1 --port 3306 -u root -e "FLUSH PRIVILEGES"

wget -O /tmp/wkhtmltox.tar.xz https://github.com/frappe/wkhtmltopdf/raw/master/wkhtmltox-0.12.3_linux-generic-amd64.tar.xz
tar -xf /tmp/wkhtmltox.tar.xz -C /tmp
sudo mv /tmp/wkhtmltox/bin/wkhtmltopdf /usr/local/bin/wkhtmltopdf
sudo chmod o+x /usr/local/bin/wkhtmltopdf
sudo apt-get install libcups2-dev

cd ~/frappe-bench || exit

sed -i 's/watch:/# watch:/g' Procfile
sed -i 's/schedule:/# schedule:/g' Procfile
sed -i 's/socketio:/# socketio:/g' Procfile
sed -i 's/redis_socketio:/# redis_socketio:/g' Procfile

bench get-app erpnext "${GITHUB_WORKSPACE}"
bench start &
bench --site test_site reinstall --yes
38 changes: 38 additions & 0 deletions .github/helper/semgrep_rules/README.md
@@ -0,0 +1,38 @@
# Semgrep linting

## What is semgrep?
Semgrep or "semantic grep" is language agnostic static analysis tool. In simple terms semgrep is syntax-aware `grep`, so unlike regex it doesn't get confused by different ways of writing same thing or whitespaces or code split in multiple lines etc.

Example:

To check if a translate function is using f-string or not the regex would be `r"_\(\s*f[\"']"` while equivalent rule in semgrep would be `_(f"...")`. As semgrep knows grammer of language it takes care of unnecessary whitespace, type of quotation marks etc.

You can read more such examples in `.github/helper/semgrep_rules` directory.

# Why/when to use this?
We want to maintain quality of contributions, at the same time remembering all the good practices can be pain to deal with while evaluating contributions. Using semgrep if you can translate "best practice" into a rule then it can automate the task for us.

## Running locally

Install semgrep using homebrew `brew install semgrep` or pip `pip install semgrep`.

To run locally use following command:

`semgrep --config=.github/helper/semgrep_rules [file/folder names]`

## Testing
semgrep allows testing the tests. Refer to this page: https://semgrep.dev/docs/writing-rules/testing-rules/

When writing new rules you should write few positive and few negative cases as shown in the guide and current tests.

To run current tests: `semgrep --test --test-ignore-todo .github/helper/semgrep_rules`


## Reference

If you are new to Semgrep read following pages to get started on writing/modifying rules:

- https://semgrep.dev/docs/getting-started/
- https://semgrep.dev/docs/writing-rules/rule-syntax
- https://semgrep.dev/docs/writing-rules/pattern-examples/
- https://semgrep.dev/docs/writing-rules/rule-ideas/#common-use-cases
28 changes: 28 additions & 0 deletions .github/helper/semgrep_rules/frappe_correctness.py
@@ -0,0 +1,28 @@
import frappe
from frappe import _, flt

from frappe.model.document import Document


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'

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")

class TestDoc(Document):
pass

def validate(self):
#ruleid: frappe-modifying-child-tables-while-iterating
for item in self.child_table:
if item.value < 0:
self.remove(item)
74 changes: 74 additions & 0 deletions .github/helper/semgrep_rules/frappe_correctness.yml
@@ -0,0 +1,74 @@
# This file specifies rules for correctness according to how frappe doctype data model works.

rules:
- id: frappe-modifying-after-submit
patterns:
- pattern: self.$ATTR = ...
- pattern-inside: |
def on_submit(self, ...):
...
- metavariable-regex:
metavariable: '$ATTR'
# this is negative look-ahead, add more attrs to ignore like (ignore|ignore_this_too|ignore_me)
regex: '^(?!status_updater)(.*)$'
message: |
Doctype modified after submission. Please check if modification of self.$ATTR is commited to database.
languages: [python]
severity: ERROR

- id: frappe-modifying-after-cancel
patterns:
- pattern: self.$ATTR = ...
- pattern-inside: |
def on_cancel(self, ...):
...
- metavariable-regex:
metavariable: '$ATTR'
regex: '^(?!ignore_linked_doctypes|status_updater)(.*)$'
message: |
Doctype modified after cancellation. Please check if modification of self.$ATTR is commited to database.
languages: [python]
severity: ERROR

- id: frappe-print-function-in-doctypes
pattern: print(...)
message: |
Did you mean to leave this print statement in? Consider using msgprint or logger instead of print statement.
languages: [python]
severity: WARNING
paths:
exclude:
- test_*.py
include:
- "*/**/doctype/*"

- id: frappe-modifying-child-tables-while-iterating
pattern-either:
- pattern: |
for $ROW in self.$TABLE:
...
self.remove(...)
- pattern: |
for $ROW in self.$TABLE:
...
self.append(...)
message: |
Child table being modified while iterating on it.
languages: [python]
severity: ERROR
paths:
include:
- "*/**/doctype/*"

- id: frappe-same-key-assigned-twice
pattern-either:
- pattern: |
{..., $X: $A, ..., $X: $B, ...}
- pattern: |
dict(..., ($X, $A), ..., ($X, $B), ...)
- pattern: |
_dict(..., ($X, $A), ..., ($X, $B), ...)
message: |
key `$X` is uselessly assigned twice. This could be a potential bug.
languages: [python]
severity: ERROR
6 changes: 6 additions & 0 deletions .github/helper/semgrep_rules/security.py
@@ -0,0 +1,6 @@
def function_name(input):
# ruleid: frappe-codeinjection-eval
eval(input)

# ok: frappe-codeinjection-eval
eval("1 + 1")
25 changes: 25 additions & 0 deletions .github/helper/semgrep_rules/security.yml
@@ -0,0 +1,25 @@
rules:
- id: frappe-codeinjection-eval
patterns:
- pattern-not: eval("...")
- pattern: eval(...)
message: |
Detected the use of eval(). eval() can be dangerous if used to evaluate
dynamic content. Avoid it or use safe_eval().
languages: [python]
severity: ERROR

- id: frappe-sqli-format-strings
patterns:
- pattern-inside: |
@frappe.whitelist()
def $FUNC(...):
...
- pattern-either:
- pattern: frappe.db.sql("..." % ...)
- pattern: frappe.db.sql(f"...", ...)
- pattern: frappe.db.sql("...".format(...), ...)
message: |
Detected use of raw string formatting for SQL queries. This can lead to sql injection vulnerabilities. Refer security guidelines - https://github.com/frappe/erpnext/wiki/Code-Security-Guidelines
languages: [python]
severity: WARNING
37 changes: 37 additions & 0 deletions .github/helper/semgrep_rules/translate.js
@@ -0,0 +1,37 @@
// ruleid: frappe-translation-empty-string
__("")
// ruleid: frappe-translation-empty-string
__('')

// ok: frappe-translation-js-formatting
__('Welcome {0}, get started with ERPNext in just a few clicks.', [full_name]);

// ruleid: frappe-translation-js-formatting
__(`Welcome ${full_name}, get started with ERPNext in just a few clicks.`);

// ok: frappe-translation-js-formatting
__('This is fine');


// ok: frappe-translation-trailing-spaces
__('This is fine');

// ruleid: frappe-translation-trailing-spaces
__(' this is not ok ');
// ruleid: frappe-translation-trailing-spaces
__('this is not ok ');
// ruleid: frappe-translation-trailing-spaces
__(' this is not ok');

// ok: frappe-translation-js-splitting
__('You have {0} subscribers in your mailing list.', [subscribers.length])

// todoruleid: frappe-translation-js-splitting
__('You have') + subscribers.length + __('subscribers in your mailing list.')

// ruleid: frappe-translation-js-splitting
__('You have' + 'subscribers in your mailing list.')

// ruleid: frappe-translation-js-splitting
__('You have {0} subscribers' +
'in your mailing list', [subscribers.length])
53 changes: 53 additions & 0 deletions .github/helper/semgrep_rules/translate.py
@@ -0,0 +1,53 @@
# Examples taken from https://frappeframework.com/docs/user/en/translations
# This file is used for testing the tests.

from frappe import _

full_name = "Jon Doe"
# ok: frappe-translation-python-formatting
_('Welcome {0}, get started with ERPNext in just a few clicks.').format(full_name)

# ruleid: frappe-translation-python-formatting
_('Welcome %s, get started with ERPNext in just a few clicks.' % full_name)
# ruleid: frappe-translation-python-formatting
_('Welcome %(name)s, get started with ERPNext in just a few clicks.' % {'name': full_name})

# ruleid: frappe-translation-python-formatting
_('Welcome {0}, get started with ERPNext in just a few clicks.'.format(full_name))


subscribers = ["Jon", "Doe"]
# ok: frappe-translation-python-formatting
_('You have {0} subscribers in your mailing list.').format(len(subscribers))

# ruleid: frappe-translation-python-splitting
_('You have') + len(subscribers) + _('subscribers in your mailing list.')

# ruleid: frappe-translation-python-splitting
_('You have {0} subscribers \
in your mailing list').format(len(subscribers))

# ok: frappe-translation-python-splitting
_('You have {0} subscribers') \
+ 'in your mailing list'

# ruleid: frappe-translation-trailing-spaces
msg = _(" You have {0} pending invoice ")
# ruleid: frappe-translation-trailing-spaces
msg = _("You have {0} pending invoice ")
# ruleid: frappe-translation-trailing-spaces
msg = _(" You have {0} pending invoice")

# ok: frappe-translation-trailing-spaces
msg = ' ' + _("You have {0} pending invoices") + ' '

# ruleid: frappe-translation-python-formatting
_(f"can not format like this - {subscribers}")
# ruleid: frappe-translation-python-splitting
_(f"what" + f"this is also not cool")


# ruleid: frappe-translation-empty-string
_("")
# ruleid: frappe-translation-empty-string
_('')
63 changes: 63 additions & 0 deletions .github/helper/semgrep_rules/translate.yml
@@ -0,0 +1,63 @@
rules:
- id: frappe-translation-empty-string
pattern-either:
- pattern: _("")
- pattern: __("")
message: |
Empty string is useless for translation.
Please refer: https://frappeframework.com/docs/user/en/translations
languages: [python, javascript, json]
severity: ERROR

- id: frappe-translation-trailing-spaces
pattern-either:
- pattern: _("=~/(^[ \t]+|[ \t]+$)/")
- pattern: __("=~/(^[ \t]+|[ \t]+$)/")
message: |
Trailing or leading whitespace not allowed in translate strings.
Please refer: https://frappeframework.com/docs/user/en/translations
languages: [python, javascript, json]
severity: ERROR

- id: frappe-translation-python-formatting
pattern-either:
- pattern: _("..." % ...)
- pattern: _("...".format(...))
- pattern: _(f"...")
message: |
Only positional formatters are allowed and formatting should not be done before translating.
Please refer: https://frappeframework.com/docs/user/en/translations
languages: [python]
severity: ERROR

- id: frappe-translation-js-formatting
patterns:
- pattern: __(`...`)
- pattern-not: __("...")
message: |
Template strings are not allowed for text formatting.
Please refer: https://frappeframework.com/docs/user/en/translations
languages: [javascript, json]
severity: ERROR

- id: frappe-translation-python-splitting
pattern-either:
- pattern: _(...) + ... + _(...)
- pattern: _("..." + "...")
- pattern-regex: '_\([^\)]*\\\s*'
message: |
Do not split strings inside translate function. Do not concatenate using translate functions.
Please refer: https://frappeframework.com/docs/user/en/translations
languages: [python]
severity: ERROR

- id: frappe-translation-js-splitting
pattern-either:
- pattern-regex: '__\([^\)]*[\+\\]\s*'
- pattern: __('...' + '...')
- pattern: __('...') + __('...')
message: |
Do not split strings inside translate function. Do not concatenate using translate functions.
Please refer: https://frappeframework.com/docs/user/en/translations
languages: [javascript, json]
severity: ERROR

0 comments on commit 4367f1c

Please sign in to comment.