Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #17600 from edx/xss-linter-configurable-skip-dirs-and-linters

Browse Source 
Configurable XSS-linter
This commit is contained in:
Anthony Mangano
2018-03-02 10:39:50 -05:00
committed by GitHub
parent 9beabad8c0 3e7df8688f
commit fc4a1b0f45
9 changed files with 207 additions and 69 deletions
Download Patch File Download Diff File Expand all files Collapse all files

3
pavelib/quality.py
View File

@@ -434,10 +434,11 @@ def run_xsslint(options):
_prepare_report_dir(xsslint_report_dir)
sh(
"{repo_root}/scripts/xsslint/{xsslint_script} --rule-totals >> {xsslint_report}".format(
"{repo_root}/scripts/xsslint/{xsslint_script} --rule-totals --config={cfg_module} >> {xsslint_report}".format(
repo_root=Env.REPO_ROOT,
xsslint_script=xsslint_script,
xsslint_report=xsslint_report,
cfg_module='scripts.xsslint_config'
),
ignore_error=True
)

2
scripts/xss-commit-linter.sh
View File

@@ -82,6 +82,6 @@ else
for f in $diff_files; do
echo ""
echo "Linting $f:"
./scripts/xsslint/xss_linter.py $f
./scripts/xsslint/xss_linter.py --config=scripts.xsslint_config $f
done
fi

74
scripts/xsslint/tests/test_linters.py
View File

@@ -15,6 +15,19 @@ from xsslint.rules import Rules
from xsslint.utils import ParseString
def _build_javascript_linter():
return JavaScriptLinter(
underscore_linter=UnderscoreTemplateLinter()
)
def _build_mako_linter():
return MakoTemplateLinter(
javascript_linter=_build_javascript_linter(),
python_linter=PythonLinter(),
)
class TestLinter(TestCase):
"""
Test Linter base class
@@ -219,7 +232,7 @@ class TestJavaScriptLinter(TestLinter):
"""
Test check_javascript_file_is_safe with concatenating strings and HTML
"""
linter = JavaScriptLinter()
linter = _build_javascript_linter()
results = FileResults('')
linter.check_javascript_file_is_safe(data['template'], results)
@@ -249,7 +262,7 @@ class TestJavaScriptLinter(TestLinter):
"""
Test check_javascript_file_is_safe with JQuery append()
"""
linter = JavaScriptLinter()
linter = _build_javascript_linter()
results = FileResults('')
linter.check_javascript_file_is_safe(data['template'], results)
@@ -277,7 +290,7 @@ class TestJavaScriptLinter(TestLinter):
"""
Test check_javascript_file_is_safe with JQuery prepend()
"""
linter = JavaScriptLinter()
linter = _build_javascript_linter()
results = FileResults('')
linter.check_javascript_file_is_safe(data['template'], results)
@@ -309,7 +322,7 @@ class TestJavaScriptLinter(TestLinter):
other than append(), prepend() and html() that take content as an
argument (e.g. before(), after()).
"""
linter = JavaScriptLinter()
linter = _build_javascript_linter()
results = FileResults('')
linter.check_javascript_file_is_safe(data['template'], results)
@@ -340,7 +353,7 @@ class TestJavaScriptLinter(TestLinter):
functions that take a target as an argument, like appendTo() and
prependTo().
"""
linter = JavaScriptLinter()
linter = _build_javascript_linter()
results = FileResults('')
linter.check_javascript_file_is_safe(data['template'], results)
@@ -364,7 +377,7 @@ class TestJavaScriptLinter(TestLinter):
"""
Test check_javascript_file_is_safe with JQuery html()
"""
linter = JavaScriptLinter()
linter = _build_javascript_linter()
results = FileResults('')
linter.check_javascript_file_is_safe(data['template'], results)
@@ -379,7 +392,7 @@ class TestJavaScriptLinter(TestLinter):
"""
Test check_javascript_file_is_safe with interpolate()
"""
linter = JavaScriptLinter()
linter = _build_javascript_linter()
results = FileResults('')
linter.check_javascript_file_is_safe(data['template'], results)
@@ -394,7 +407,7 @@ class TestJavaScriptLinter(TestLinter):
"""
Test check_javascript_file_is_safe with interpolate()
"""
linter = JavaScriptLinter()
linter = _build_javascript_linter()
results = FileResults('')
linter.check_javascript_file_is_safe(data['template'], results)
@@ -737,7 +750,8 @@ class TestMakoTemplateLinter(TestLinter):
"""
Test _is_valid_directory correctly determines mako directories
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
linter._skip_mako_dirs = ('test_root',)
self.assertEqual(linter._is_valid_directory(data['directory']), data['expected'])
@@ -785,7 +799,7 @@ class TestMakoTemplateLinter(TestLinter):
"""
Test _check_mako_file_is_safe with different page defaults
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
linter._check_mako_file_is_safe(data['template'], results)
@@ -808,7 +822,7 @@ class TestMakoTemplateLinter(TestLinter):
"""
Test _check_mako_file_is_safe in html context provides appropriate violations
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -825,7 +839,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe with display_name_with_default_escaped
fails.
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -971,7 +985,7 @@ class TestMakoTemplateLinter(TestLinter):
"""
Test _check_mako_file_is_safe tests for proper use of Text() and Html().
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -988,7 +1002,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe does not fail on entities when
safe-by-default is not set.
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = "${'Rock & Roll'}"
@@ -1003,7 +1017,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe with disable pragma for safe-by-default
works to designate that this is not a Mako file
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1023,7 +1037,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe with disable pragma results in no
violation
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1047,7 +1061,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe with disable pragma results in no
violation
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
linter._check_mako_file_is_safe(data['template'], results)
@@ -1059,7 +1073,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe results in no violations,
when strip_all_tags_but_br filter is applied in html context
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1075,7 +1089,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe in html context without the page level
default h filter suppresses expression level violation
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1100,7 +1114,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe in JavaScript script context provides
appropriate violations
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1126,7 +1140,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe in JavaScript require context provides
appropriate violations
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1152,7 +1166,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe in JavaScript require js context provides
appropriate violations
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1183,7 +1197,7 @@ class TestMakoTemplateLinter(TestLinter):
"""
Test _check_mako_file_is_safe in script tag with different media types
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1205,7 +1219,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe in mixed contexts provides
appropriate violations
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1242,7 +1256,7 @@ class TestMakoTemplateLinter(TestLinter):
- mako_js_missing_quotes
- mako_js_html_string
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1277,7 +1291,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe with JavaScript error in JavaScript
context.
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
mako_template = textwrap.dedent("""
@@ -1311,7 +1325,7 @@ class TestMakoTemplateLinter(TestLinter):
Test _check_mako_file_is_safe provides detailed results, including line
numbers, columns, and line
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
results = FileResults('')
linter._check_mako_file_is_safe(data['template'], results)
@@ -1346,7 +1360,7 @@ class TestMakoTemplateLinter(TestLinter):
"""
Test _find_mako_expressions for parseable expressions
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
expressions = linter._find_mako_expressions(data['template'])
@@ -1364,7 +1378,7 @@ class TestMakoTemplateLinter(TestLinter):
"""
Test _find_mako_expressions for unparseable expressions
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
expressions = linter._find_mako_expressions(data['template'])
self.assertTrue(2 <= len(expressions))
@@ -1401,7 +1415,7 @@ class TestMakoTemplateLinter(TestLinter):
"""
Test _parse_string helper
"""
linter = MakoTemplateLinter()
linter = _build_mako_linter()
parse_string = ParseString(data['template'], data['result']['start_index'], len(data['template']))
string_dict = {

20
scripts/xsslint/tests/test_main.py
View File

@@ -46,6 +46,13 @@ class TestXSSLinter(TestCase):
self.addCleanup(patcher.stop)
return patch_start
def _build_linters(self):
underscore_linter = UnderscoreTemplateLinter()
python_linter = PythonLinter()
javascript_linter = JavaScriptLinter(underscore_linter=underscore_linter)
mako_linter = MakoTemplateLinter(javascript_linter=javascript_linter, python_linter=python_linter)
return [mako_linter, underscore_linter, javascript_linter, python_linter]
def test_lint_defaults(self):
"""
Tests the top-level linting with default options.
@@ -55,11 +62,12 @@ class TestXSSLinter(TestCase):
_lint(
'scripts/xsslint/tests/templates',
template_linters=[MakoTemplateLinter(), UnderscoreTemplateLinter(), JavaScriptLinter(), PythonLinter()],
template_linters=self._build_linters(),
options={
'list_files': False,
'verbose': False,
'rule_totals': False,
'skip_dirs': ()
},
summary_results=summary_results,
out=out,
@@ -97,11 +105,12 @@ class TestXSSLinter(TestCase):
_lint(
'scripts/xsslint/tests/templates',
template_linters=[MakoTemplateLinter(), UnderscoreTemplateLinter(), JavaScriptLinter(), PythonLinter()],
template_linters=self._build_linters(),
options={
'list_files': False,
'verbose': True,
'rule_totals': False,
'skip_dirs': ()
},
summary_results=summary_results,
out=out,
@@ -131,11 +140,12 @@ class TestXSSLinter(TestCase):
_lint(
'scripts/xsslint/tests/templates',
template_linters=[MakoTemplateLinter(), UnderscoreTemplateLinter(), JavaScriptLinter(), PythonLinter()],
template_linters=self._build_linters(),
options={
'list_files': False,
'verbose': False,
'rule_totals': True,
'skip_dirs': ()
},
summary_results=summary_results,
out=out,
@@ -155,14 +165,14 @@ class TestXSSLinter(TestCase):
"""
out = StringIO()
summary_results = SummaryResults()
_lint(
'scripts/xsslint/tests/templates',
template_linters=[MakoTemplateLinter(), UnderscoreTemplateLinter(), JavaScriptLinter(), PythonLinter()],
template_linters=self._build_linters(),
options={
'list_files': True,
'verbose': False,
'rule_totals': False,
'skip_dirs': ()
},
summary_results=summary_results,
out=out,

51
scripts/xsslint/xsslint/default_config.py Normal file
View File

@@ -0,0 +1,51 @@
# Default xsslint config module.
from xsslint.linters import JavaScriptLinter, MakoTemplateLinter, PythonLinter, UnderscoreTemplateLinter
# Define the directories that should be ignored by the script.
SKIP_DIRS = (
'.git',
'.pycharm_helpers',
'common/static/xmodule/modules',
'common/static/bundles',
'perf_tests',
'node_modules',
'reports/diff_quality',
'scripts/xsslint',
'spec',
'test_root',
'vendor',
)
UNDERSCORE_SKIP_DIRS = SKIP_DIRS + ('test',)
UNDERSCORE_LINTER = UnderscoreTemplateLinter(
skip_dirs=UNDERSCORE_SKIP_DIRS
)
JAVASCRIPT_SKIP_DIRS = SKIP_DIRS + ('i18n', 'static/coffee')
COFFEESCRIPT_SKIP_DIRS = SKIP_DIRS
JAVASCRIPT_LINTER = JavaScriptLinter(
underscore_linter=UNDERSCORE_LINTER,
javascript_skip_dirs=JAVASCRIPT_SKIP_DIRS,
coffeescript_skip_dirs=COFFEESCRIPT_SKIP_DIRS
)
PYTHON_SKIP_DIRS = SKIP_DIRS + ('tests', 'test/acceptance')
PYTHON_LINTER = PythonLinter(
skip_dirs=PYTHON_SKIP_DIRS
)
MAKO_SKIP_DIRS = SKIP_DIRS
MAKO_LINTER = MakoTemplateLinter(
javascript_linter=JAVASCRIPT_LINTER,
python_linter=PYTHON_LINTER,
skip_dirs=MAKO_SKIP_DIRS
)
# (Required) Define the linters (code-checkers) that should be run by the script.
LINTERS = (MAKO_LINTER, UNDERSCORE_LINTER, JAVASCRIPT_LINTER, PYTHON_LINTER)

27
scripts/xsslint/xsslint/linters.py
View File

@@ -8,7 +8,7 @@ import textwrap
from xsslint.reporting import ExpressionRuleViolation, FileResults, RuleViolation
from xsslint.rules import Rules
from xsslint.utils import SKIP_DIRS, Expression, ParseString, StringLines, is_skip_dir
from xsslint.utils import Expression, ParseString, StringLines, is_skip_dir
from xsslint.visitors import AllNodeVisitor, HtmlStringVisitor, OuterFormatVisitor
@@ -194,12 +194,12 @@ class UnderscoreTemplateLinter(BaseLinter):
"""
The linter for Underscore.js template files.
"""
def __init__(self):
def __init__(self, skip_dirs=None):
"""
Init method.
"""
super(UnderscoreTemplateLinter, self).__init__()
self._skip_underscore_dirs = SKIP_DIRS + ('test',)
self._skip_underscore_dirs = skip_dirs or ()
def process_file(self, directory, file_name):
"""
@@ -309,14 +309,14 @@ class JavaScriptLinter(BaseLinter):
LINE_COMMENT_DELIM = "//"
def __init__(self):
def __init__(self, underscore_linter, javascript_skip_dirs=None, coffeescript_skip_dirs=None):
"""
Init method.
"""
super(JavaScriptLinter, self).__init__()
self._skip_javascript_dirs = SKIP_DIRS + ('i18n', 'static/coffee')
self._skip_coffeescript_dirs = SKIP_DIRS
self.underscore_linter = UnderscoreTemplateLinter()
self.underscore_linter = underscore_linter
self._skip_javascript_dirs = javascript_skip_dirs or ()
self._skip_coffeescript_dirs = coffeescript_skip_dirs or ()
def process_file(self, directory, file_name):
"""
@@ -697,12 +697,12 @@ class PythonLinter(BaseLinter):
LINE_COMMENT_DELIM = "#"
def __init__(self):
def __init__(self, skip_dirs=None):
"""
Init method.
"""
super(PythonLinter, self).__init__()
self._skip_python_dirs = SKIP_DIRS + ('tests', 'test/acceptance')
self._skip_python_dirs = skip_dirs or ()
def process_file(self, directory, file_name):
"""
@@ -856,13 +856,14 @@ class MakoTemplateLinter(BaseLinter):
"""
LINE_COMMENT_DELIM = "##"
def __init__(self):
def __init__(self, javascript_linter, python_linter, skip_dirs=None):
"""
Init method.
"""
super(MakoTemplateLinter, self).__init__()
self.javascript_linter = JavaScriptLinter()
self.python_linter = PythonLinter()
self.javascript_linter = javascript_linter
self.python_linter = python_linter
self._skip_mako_dirs = skip_dirs or ()
def process_file(self, directory, file_name):
"""
@@ -909,7 +910,7 @@ class MakoTemplateLinter(BaseLinter):
True if this directory should be linted for Mako template violations
and False otherwise.
"""
if is_skip_dir(SKIP_DIRS, directory):
if is_skip_dir(self._skip_mako_dirs, directory):
return False
# TODO: This is an imperfect guess concerning the Mako template

26
scripts/xsslint/xsslint/main.py
View File

@@ -2,12 +2,20 @@
The main function for the XSS linter.
"""
import argparse
import importlib
import os
import sys
from xsslint.linters import JavaScriptLinter, MakoTemplateLinter, PythonLinter, UnderscoreTemplateLinter
from xsslint.reporting import SummaryResults
from xsslint.utils import SKIP_DIRS, is_skip_dir
from xsslint.utils import is_skip_dir
def _load_config_module(module_path):
cwd = os.getcwd()
if cwd not in sys.path:
# Enable config module to be imported relative to wherever the script was run from.
sys.path.append(cwd)
return importlib.import_module(module_path)
def _process_file(full_path, template_linters, options, summary_results, out):
@@ -61,8 +69,9 @@ def _process_os_dirs(starting_dir, template_linters, options, summary_results, o
out: output file
"""
skip_dirs = options.get('skip_dirs', ())
for root, dirs, files in os.walk(starting_dir):
if is_skip_dir(SKIP_DIRS, root):
if is_skip_dir(skip_dirs, root):
del dirs
continue
dirs.sort(key=lambda s: s.lower())
@@ -125,16 +134,23 @@ def main():
'--verbose', dest='verbose', action='store_true',
help='Print multiple lines where possible for additional context of violations.'
)
parser.add_argument(
'--config', dest='config', action='store', default='xsslint.default_config',
help='Specifies the config module to use. The config module should be in Python package syntax.'
)
parser.add_argument('path', nargs="?", default=None, help='A file to lint or directory to recursively lint.')
args = parser.parse_args()
config = _load_config_module(args.config)
options = {
'list_files': args.list_files,
'rule_totals': args.rule_totals,
'verbose': args.verbose,
'skip_dirs': getattr(config, 'SKIP_DIRS', ())
}
template_linters = [MakoTemplateLinter(), UnderscoreTemplateLinter(), JavaScriptLinter(), PythonLinter()]
summary_results = SummaryResults()
template_linters = getattr(config, 'LINTERS', ())
if not template_linters:
raise ValueError("LINTERS is empty or undefined in the config module ({}).".format(args.config))
_lint(args.path, template_linters, options, summary_results, out=sys.stdout)

14
scripts/xsslint/xsslint/utils.py
View File

@@ -3,20 +3,6 @@ Utility classes/functions for the XSS Linter.
"""
import re
SKIP_DIRS = (
'.git',
'.pycharm_helpers',
'common/static/xmodule/modules',
'common/static/bundles',
'perf_tests',
'node_modules',
'reports/diff_quality',
'scripts/xsslint',
'spec',
'test_root',
'vendor',
)
def is_skip_dir(skip_dirs, directory):
"""

59
scripts/xsslint_config.py Normal file
View File

@@ -0,0 +1,59 @@
# xsslint config module for edx-platform
import os
import sys
# Temporarily add xsslint to sys.path so that we can import from it. This won't be necessary once
# xsslint is moved out of edx-platform.
scripts_dir = os.path.dirname(os.path.abspath(__file__))
sys.path.append(os.path.join(scripts_dir, 'xsslint'))
from xsslint.linters import JavaScriptLinter, MakoTemplateLinter, PythonLinter, UnderscoreTemplateLinter
# Define the directories that should be ignored by the script.
SKIP_DIRS = (
'.git',
'.pycharm_helpers',
'common/static/xmodule/modules',
'common/static/bundles',
'perf_tests',
'node_modules',
'reports/diff_quality',
'scripts/xsslint',
'spec',
'test_root',
'vendor',
)
UNDERSCORE_SKIP_DIRS = SKIP_DIRS + ('test',)
UNDERSCORE_LINTER = UnderscoreTemplateLinter(
skip_dirs=UNDERSCORE_SKIP_DIRS
)
JAVASCRIPT_SKIP_DIRS = SKIP_DIRS + ('i18n', 'static/coffee')
COFFEESCRIPT_SKIP_DIRS = SKIP_DIRS
JAVASCRIPT_LINTER = JavaScriptLinter(
underscore_linter=UNDERSCORE_LINTER,
javascript_skip_dirs=JAVASCRIPT_SKIP_DIRS,
coffeescript_skip_dirs=COFFEESCRIPT_SKIP_DIRS
)
PYTHON_SKIP_DIRS = SKIP_DIRS + ('tests', 'test/acceptance')
PYTHON_LINTER = PythonLinter(
skip_dirs=PYTHON_SKIP_DIRS
)
MAKO_SKIP_DIRS = SKIP_DIRS
MAKO_LINTER = MakoTemplateLinter(
javascript_linter=JAVASCRIPT_LINTER,
python_linter=PYTHON_LINTER,
skip_dirs=MAKO_SKIP_DIRS
)
# (Required) Define the linters (code-checkers) that should be run by the script.
LINTERS = (MAKO_LINTER, UNDERSCORE_LINTER, JAVASCRIPT_LINTER, PYTHON_LINTER)