ishangsf/rspec
2
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

RULEAPI-766 Add documentation and integrity checks for new education rule descriptions format (#1098)

Browse Source
This commit is contained in:
Christophe Zürn 2022-07-07 14:06:43 +02:00
parent 7c36d2a006
commit 47ba59f3b5
19 changed files with 336 additions and 11 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
docs/allowed_framework_names.adoc Normal file
View File

@ -0,0 +1,19 @@
// C#
* ASP.NET
* Razor
// Java
* JSP
* Servlet
* Spring
* Thymeleaf
// JS
* Express.js
// PHP
* Core PHP
* Laravel
* Symfony
// Python
* Django
* Django Templates
* Flask
* Jinja

111
docs/description.adoc
View File

@ -12,7 +12,116 @@ The following are the only allowed second level titles for H2:
include::section_names.adoc[]
Third level titles are not checked.
Most third-level titles (`+=== Title t3+`) are not checked, the only type of rule description for which some of these
titles are checked are the progressive education rules (see below).
== Types of rule description
The above list of allowed H2 sections is an absolute list and covers all the different types of rule descriptions that exist.
However, depending on the type of rule description, the expected order and allowed sections are not the same.
There are currently 3 types of rule descriptions, each having a specific structure.
=== 1. Standard
The standard rule description is the currently most widespread format. It includes all _Code Smells_, _Bugs_ and most _Vulnerabilities_.
This is the "classical" format that starts with a description of the rule, and is usually followed by code examples of bad and good practices,
with some links to resources at the end.
This format has the structure of the following section:
. Description (no title)
. Noncompliant Code Example
. Compliant Solution
. Exceptions
. See
. See Also
Note that most sections are optional, only the description is mandatory.
=== 2. Hotspot
Specific to the _Hotspot_ issue type, this format is mostly similar to the standard one,
with the addition of 2 new sections: `Ask Yourself Whether` and `Recommended Secure Coding Practices`. Additionally, the `Noncompliant Code Example`
section has been renamed into `Sensitive Code Example`: this is based on the fact that for _Hotspots_ there is no certainty that what was raised
is an actual vulnerability.
The possible sections for this format are the following:
. Description (no title)
. Ask Yourself Whether
. Recommended Secure Coding Practices
. Sensitive Code Example
. Compliant Solution
. Exceptions
. See
. See Also
=== 3. Progressive Education
Finally, the last format is the one related to the effort done on Progressive Education.
This new format has a completely new structure, with additional subtitles and information,
so as to be as comprehensible as possible for the users.
This new structure is defined as follows:
. Short description (no title)
. Why is this an issue?
.. What is the potential impact?
. How to fix it?
.. How to fix it in `{Framework Display Name}`
.. How does this work?
.. Pitfalls
.. Going the extra mile
. Resources
.. Documentation
.. Articles & Blog Posts
.. Conference Presentations
.. Standards
Content of the section "_3. How to fix it?_" (i.e. subsections 3.a, 3.b, 3.c and 3.d) may be repeated multiple times, as
each repetition will represent the specific _How to fix it?_ section for a given framework.
For example:
....
== How to fix it?
=== How to fix it in Spring
... Some generic text and code examples for Spring...
=== How does this work?
... Explanation about how the exploit works in Spring...
=== Pitfalls
... Generic and Spring-specific pitfalls to avoid when fixing the issue...
=== How to fix it in JSP
... Some generic text and code examples for JSP...
=== How does this work?
... Explanation about how the exploit works in JSP...
=== Pitfalls
... Generic and JSP-specific pitfalls to avoid when fixing the issue...
....
Ideally, by convention and for maintainability, each framework _How to fix it?_ section will be defined in separate files.
Ex:
....
== How to fix it?
include::./how-to-fix-it/framework-1.adoc[]
include::./how-to-fix-it/framework-2.adoc[]
....
Note that each framework-specific _How to fix it?_ subsection must start with an H2 subtitle following the given format:
`How to fix it in (?:(?:an|a|the)\\s)?(.*)`.
This is important, as this format will be expected by the analyzers when loading the rule content to recognize the different subsections.
Furthermore, the display name of the framework has to match an allowed framework
display name, as defined in <<allowed_framework_names.adoc#,this allowed framework names file>>.
Finally, additional information regarding each of the sections and subsections present in the educational rule description
can be found https://docs.google.com/document/d/1caqBXTO1r7vftWrNXt_aNVe4rHcB5C6u6oKMBFjbzwo[in this document].
== Code Examples

6
rspec-tools/rspec_tools/cli.py
View File

@ -13,7 +13,8 @@ from rspec_tools.coverage import (update_coverage_for_all_repos,
from rspec_tools.errors import RuleValidationError
from rspec_tools.notify_failure_on_slack import notify_slack
from rspec_tools.rules import LanguageSpecificRule, RulesRepository
from rspec_tools.validation.description import (validate_parameters,
from rspec_tools.validation.description import (validate_how_to_fix_it_subsections,
validate_parameters,
validate_section_levels,
validate_section_names,
validate_source_language)
@ -86,7 +87,8 @@ def validate_rules_metadata(rules):
_fatal_error(f"Validation failed due to {error_counter} errors out of {len(rules)} analyzed rules")
VALIDATORS = [validate_section_names,
VALIDATORS = [validate_how_to_fix_it_subsections,
validate_section_names,
validate_section_levels,
validate_parameters,
validate_source_language,

30
rspec-tools/rspec_tools/validation/description.py
View File

@ -6,6 +6,8 @@ from rspec_tools.errors import RuleValidationError
from rspec_tools.rules import LanguageSpecificRule
from rspec_tools.utils import LANG_TO_SOURCE
import re
# The list of all the sections currently accepted by the script.
# The list includes multiple variants for each title because they all occur
# in the migrated RSPECs.
@ -14,6 +16,11 @@ from rspec_tools.utils import LANG_TO_SOURCE
SECTION_NAMES_PATH = Path(__file__).parent.parent.parent.parent.joinpath('docs/section_names.adoc')
SECTION_NAMES_FILE = SECTION_NAMES_PATH.read_text(encoding='utf-8').split('\n')
ACCEPTED_SECTION_NAMES: Final[list[str]] = [s.replace('* ', '').strip() for s in SECTION_NAMES_FILE if s.strip()]
# The list of all the framework names currently accepted by the script.
FRAMEWORK_NAMES_PATH = Path(__file__).parent.parent.parent.parent.joinpath('docs/allowed_framework_names.adoc')
FRAMEWORK_NAMES_FILE = FRAMEWORK_NAMES_PATH.read_text(encoding='utf-8').split('\n')
ACCEPTED_FRAMEWORK_NAMES: Final[list[str]] = [s.replace('* ', '').strip() for s in FRAMEWORK_NAMES_FILE if s.strip()]
def validate_section_names(rule_language: LanguageSpecificRule):
descr = rule_language.description
for h2 in descr.find_all('h2'):
@ -21,6 +28,29 @@ def validate_section_names(rule_language: LanguageSpecificRule):
if name not in ACCEPTED_SECTION_NAMES:
raise RuleValidationError(f'Rule {rule_language.id} has unconventional header "{name}"')
def validate_how_to_fix_it_subsections(rule_language: LanguageSpecificRule):
descr = rule_language.description
frameworks_counter = 0
for h3 in descr.find_all('h3'):
name = h3.text.strip()
# It is important that the Regex here matches the one used by the analyzers when loading the rules content
result = re.search('How to fix it in (?:(?:an|a|the)\\s)?(.*)', name)
if result is not None:
if result.group(1) not in ACCEPTED_FRAMEWORK_NAMES:
raise RuleValidationError(f'Rule {rule_language.id} has a "How to fix it" section for an unsupported framework: "{result.group(1)}"')
else:
frameworks_counter += 1
how_to_fix_it_section = descr.find('h2', string='How to fix it?')
if how_to_fix_it_section is not None:
if frameworks_counter == 0:
raise RuleValidationError(f'Rule {rule_language.id} has a "How to fix it" section but is missing subsections related to frameworks')
if frameworks_counter > 6:
raise RuleValidationError(f'Rule {rule_language.id} has more than 6 "How to fix it" subsections. Please ensure this limit can be increased with PM/UX teams')
elif frameworks_counter > 0:
raise RuleValidationError(f'Rule {rule_language.id} has "How to fix it" subsections for frameworks outside a defined "How to fix it?" section')
def validate_section_levels(rule_language: LanguageSpecificRule):
h1 = rule_language.description.find('h1')
if h1 is not None:

2
rspec-tools/tests/resources/invalid-rules/S101/csharp/rule.adoc Normal file
View File

@ -0,0 +1,2 @@
== How to fix it?
=== How to fix it in Foo Bar Framework

15
rspec-tools/tests/resources/invalid-rules/S101/csharp/rule.html Normal file
View File

@ -0,0 +1,15 @@
<body class="article">
<div id="header">
</div>
<div id="content">
<div class="sect1">
<h2 id="_how_to_fix_it">How to fix it?</h2>
<div class="sectionbody">
<div class="sect2">
<h3 id="_how_to_fix_it_in_foo_bar_framework">How to fix it in Foo Bar Framework</h3>
</div>
</div>
</div>
</div>
</body>

1
rspec-tools/tests/resources/invalid-rules/S101/java/rule.adoc Normal file
View File

@ -0,0 +1 @@
== How to fix it?

11
rspec-tools/tests/resources/invalid-rules/S101/java/rule.html Normal file
View File

@ -0,0 +1,11 @@
<body class="article">
<div id="header">
</div>
<div id="content">
<div class="sect1">
<h2 id="_how_to_fix_it">How to fix it?</h2>
<div class="sectionbody">
</div>
</div>
</div>
</body>

8
rspec-tools/tests/resources/invalid-rules/S101/javascript/rule.adoc Normal file
View File

@ -0,0 +1,8 @@
== How to fix it?
=== How to fix it in Express.js
=== How to fix it in Express.js
=== How to fix it in Express.js
=== How to fix it in Express.js
=== How to fix it in Express.js
=== How to fix it in Express.js
=== How to fix it in Express.js

39
rspec-tools/tests/resources/invalid-rules/S101/javascript/rule.html Normal file
View File

@ -0,0 +1,39 @@
<body class="article">
<div id="header">
</div>
<div id="content">
<div class="sect1">
<h2 id="_how_to_fix_it">How to fix it?</h2>
<div class="sectionbody">
<div class="sect2">
<h3 id="_how_to_fix_it_in_express_js">How to fix it in Express.js</h3>
</div>
<div class="sect2">
<h3 id="_how_to_fix_it_in_express_js_2">How to fix it in Express.js</h3>
</div>
<div class="sect2">
<h3 id="_how_to_fix_it_in_express_js_3">How to fix it in Express.js</h3>
</div>
<div class="sect2">
<h3 id="_how_to_fix_it_in_express_js_4">How to fix it in Express.js</h3>
</div>
<div class="sect2">
<h3 id="_how_to_fix_it_in_express_js_5">How to fix it in Express.js</h3>
</div>
<div class="sect2">
<h3 id="_how_to_fix_it_in_express_js_6">How to fix it in Express.js</h3>
</div>
<div class="sect2">
<h3 id="_how_to_fix_it_in_express_js_7">How to fix it in Express.js</h3>
</div>
</div>
</div>
</div>
</body>

1
rspec-tools/tests/resources/invalid-rules/S101/python/rule.adoc Normal file
View File

@ -0,0 +1 @@
=== How to fix it in Flask

10
rspec-tools/tests/resources/invalid-rules/S101/python/rule.html Normal file
View File

@ -0,0 +1,10 @@
<body class="article">
<div id="header">
</div>
<div id="content">
<div class="sect2">
<h3 id="_how_to_fix_it_in_flask">How to fix it in Flask</h3>
</div>
</div>
</body>

3
rspec-tools/tests/resources/rules/S101/csharp/rule.adoc Normal file
View File

@ -0,0 +1,3 @@
== How to fix it?
=== How to fix it in ASP.NET
=== How to fix it in Razor

19
rspec-tools/tests/resources/rules/S101/csharp/rule.html Normal file
View File

@ -0,0 +1,19 @@
<body class="article">
<div id="header">
</div>
<div id="content">
<div class="sect1">
<h2 id="_how_to_fix_it">How to fix it?</h2>
<div class="sectionbody">
<div class="sect2">
<h3 id="_how_to_fix_it_in_asp_net">How to fix it in ASP.NET</h3>
</div>
<div class="sect2">
<h3 id="_how_to_fix_it_in_razor">How to fix it in Razor</h3>
</div>
</div>
</div>
</div>
</body>

28
rspec-tools/tests/resources/rules/S101/metadata.json Normal file
View File

@ -0,0 +1,28 @@
{
"title": "Class names should comply with a naming convention",
"type": "CODE_SMELL",
"status": "ready",
"remediation": {
"func": "Constant\/Issue",
"constantCost": "5min"
},
"tags": [
"convention"
],
"extra": {
"replacementRules": [
],
"legacyKeys": [
"ClassName"
]
},
"defaultSeverity": "Minor",
"ruleSpecification": "RSPEC-101",
"sqKey": "S101",
"scope": "All",
"defaultQualityProfiles": [
"Sonar way"
],
"quickfix": "unknown"
}

2
rspec-tools/tests/test_create_rule.py
View File

@ -217,7 +217,7 @@ def test_add_language_the_rule_is_already_defined_for(rule_creator: RuleCreator)
def test_add_language_to_nonexistent_rule(rule_creator: RuleCreator):
'''Test add_language_branch correctly fails when invoked for a non-existent rule.'''
with pytest.raises(InvalidArgumentError):
rule_creator.add_language_branch(101, 'cfamily')
rule_creator.add_language_branch(105, 'cfamily')
def test_add_language_new_pull_request(rule_creator: RuleCreator):

2
rspec-tools/tests/test_rules_repository.py
View File

@ -7,7 +7,7 @@ from rspec_tools.errors import RuleNotFoundError
def test_list_rules(mockrules: Path):
'''Check that rules are all listed.'''
rules = {rule.id for rule in RulesRepository(rules_path=mockrules).rules}
assert rules == {'S100', 'S120', 'S4727', 'S1033'}
assert rules == {'S100', 'S101', 'S120', 'S4727', 'S1033'}
def test_list_languages(mockrules: Path):

38
rspec-tools/tests/validation/test_description_validation.py
View File

@ -1,13 +1,12 @@
import re
from pathlib import Path
from unittest.mock import patch, PropertyMock
import pytest
import re
from rspec_tools.errors import RuleValidationError
from copy import deepcopy
from rspec_tools.rules import RulesRepository
from rspec_tools.validation.description import validate_how_to_fix_it_subsections, validate_section_names, \
validate_section_levels, validate_parameters, validate_source_language
from rspec_tools.rules import LanguageSpecificRule, RulesRepository
from rspec_tools.validation.description import validate_section_names, validate_section_levels, validate_parameters, validate_source_language
@pytest.fixture
def rule_language(mockrules: Path):
@ -87,3 +86,32 @@ def test_wrong_source_language_fails_validation(invalid_rule):
rule = invalid_rule('S100', 'csharp')
with pytest.raises(RuleValidationError, match=re.escape(f'Rule {rule.id} has unknown language "unknown" in code example in section "Noncompliant Code Example".\nAre you looking for "csharp"?')):
validate_source_language(rule)
def test_unsupported_framework_name_in_how_to_fix_it_subsection_validation(invalid_rule):
'''Check that having "How to fix it" subsections using framework names that are not inside the "allowed_framework_names.adoc" file breaks validation'''
rule = invalid_rule('S101', 'csharp')
with pytest.raises(RuleValidationError, match=f'Rule csharp:S101 has a "How to fix it" section for an unsupported framework: "Foo Bar Framework"'):
validate_how_to_fix_it_subsections(rule)
def test_missing_subsections_in_how_to_fix_it_validation(invalid_rule):
'''Check that having a "How to fix it?" section without any subsection breaks validation'''
rule = invalid_rule('S101', 'java')
with pytest.raises(RuleValidationError, match=f'Rule java:S101 has a "How to fix it" section but is missing subsections related to frameworks'):
validate_how_to_fix_it_subsections(rule)
def test_too_many_subsections_in_how_to_fix_it_validation(invalid_rule):
'''Check that having more than the current hard limit (6) "How to fix it" subsections breaks validation'''
rule = invalid_rule('S101', 'javascript')
with pytest.raises(RuleValidationError, match=f'Rule javascript:S101 has more than 6 "How to fix it" subsections. Please ensure this limit can be increased with PM/UX teams'):
validate_how_to_fix_it_subsections(rule)
def test_subsections_without_parent_section_in_how_to_fix_it_validation(invalid_rule):
'''Check that having "How to fix it" subsections without the parent "How to fix it?" section breaks validation'''
rule = invalid_rule('S101', 'python')
with pytest.raises(RuleValidationError, match=f'Rule python:S101 has "How to fix it" subsections for frameworks outside a defined "How to fix it\\?" section'):
validate_how_to_fix_it_subsections(rule)
def test_valid_how_to_fix_it_subsections_validation(rule_language):
'''Check that expected format is considered valid'''
rule = rule_language('S101', 'csharp')
validate_how_to_fix_it_subsections(rule)

2
rules/S5131/python/how-to-fix-it/dtl.adoc
View File

@ -1,4 +1,4 @@
=== How to fix it in DTL
=== How to fix it in Django Templates
The following code is vulnerable to cross-site scripting because auto-escaping of special HTML characters has been disabled. The recommended way to fix this code is to move the HTML content to the template and to only inject the dynamic value. Therefore, it is not necessary to disable auto-escaping.