Skip to content

Commit dec6bd3

Browse files
Jannis-MittenzweiNicoretti
Jannis-Mittenzwei
and
Nicoretti
authored
Add security linter results to workflow summary (#254)
--------- Co-authored-by: Nicola Coretti <nicola.coretti@exasol.com>
1 parent 3e1c23e commit dec6bd3

File tree

8 files changed

+312
-78
lines changed

8 files changed

+312
-78
lines changed

‎.github/workflows/report.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,4 @@ jobs:
5151
poetry run coverage report -- --format markdown >> $GITHUB_STEP_SUMMARY
5252
echo -e "\n\n# Static Code Analysis\n" >> $GITHUB_STEP_SUMMARY
5353
cat .lint.txt >> $GITHUB_STEP_SUMMARY
54+
poetry run tbx security pretty-print .security.json >> $GITHUB_STEP_SUMMARY

‎doc/changes/unreleased.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,6 @@
22

33
## ✨ Added
44

5-
* #233: Added nox task to verify dependency declarations
5+
* #248: Added security results to workflow summary
6+
* #233: Added nox task to verify dependency declarations
7+

‎exasol/toolbox/templates/github/workflows/report.yml‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,4 @@ jobs:
5151
poetry run coverage report -- --format markdown >> $GITHUB_STEP_SUMMARY
5252
echo -e "\n\n# Static Code Analysis\n" >> $GITHUB_STEP_SUMMARY
5353
cat .lint.txt >> $GITHUB_STEP_SUMMARY
54+
poetry run tbx security pretty-print .security.json >> $GITHUB_STEP_SUMMARY

‎exasol/toolbox/tools/security.py‎

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@
1616
Iterable,
1717
Tuple,
1818
)
19-
2019
import typer
20+
from pathlib import Path
2121

2222
stdout = print
2323
stderr = partial(print, file=sys.stderr)
@@ -101,6 +101,64 @@ def from_maven(report: str) -> Iterable[Issue]:
101101
)
102102

103103

104+
@dataclass(frozen=True)
105+
class SecurityIssue:
106+
file_name: str
107+
line: int
108+
column: int
109+
cwe: str
110+
test_id: str
111+
description: str
112+
references: tuple
113+
114+
115+
def from_json(report_str: str, prefix: Path) -> Iterable[SecurityIssue]:
116+
report = json.loads(report_str)
117+
issues = report.get("results", {})
118+
for issue in issues:
119+
references = []
120+
if issue["more_info"]:
121+
references.append(issue["more_info"])
122+
if issue.get("issue_cwe", {}).get("link", None):
123+
references.append(issue["issue_cwe"]["link"])
124+
yield SecurityIssue(
125+
file_name=issue["filename"].replace(str(prefix) + "/", ""),
126+
line=issue["line_number"],
127+
column=issue["col_offset"],
128+
cwe=str(issue["issue_cwe"].get("id", "")),
129+
test_id=issue["test_id"],
130+
description=issue["issue_text"],
131+
references=tuple(references)
132+
)
133+
134+
135+
def issues_to_markdown(issues: Iterable[SecurityIssue]) -> str:
136+
template = cleandoc("""
137+
{header}{rows}
138+
""")
139+
140+
def _header():
141+
header = "# Security\n\n"
142+
header += "|File|line/<br>column|Cwe|Test ID|Details|\n"
143+
header += "|---|:-:|:-:|:-:|---|\n"
144+
return header
145+
146+
def _row(issue):
147+
row = "|" + issue.file_name + "|"
148+
row += f"line: {issue.line}<br>column: {issue.column}|"
149+
row += issue.cwe + "|"
150+
row += issue.test_id + "|"
151+
for element in issue.references:
152+
row += element + " ,<br>"
153+
row = row[:-5] + "|"
154+
return row
155+
156+
return template.format(
157+
header=_header(),
158+
rows="\n".join(_row(i) for i in issues)
159+
)
160+
161+
104162
def security_issue_title(issue: Issue) -> str:
105163
return f"🔐 {issue.cve}: {issue.coordinates}"
106164

@@ -159,7 +217,6 @@ def create_security_issue(issue: Issue, project="") -> Tuple[str, str]:
159217
CVE_CLI = typer.Typer()
160218
CLI.add_typer(CVE_CLI, name="cve", help="Work with CVE's")
161219

162-
163220
class Format(str, Enum):
164221
Maven = "maven"
165222

@@ -257,6 +314,21 @@ def create(
257314
stdout(format_jsonl(issue_url, issue))
258315

259316

317+
class PPrintFormats(str, Enum):
318+
markdown = "markdown"
319+
320+
321+
@CLI.command(name="pretty-print")
322+
def json_issue_to_markdown(
323+
json_file: typer.FileText = typer.Argument(mode="r", help="json file with issues to convert"),
324+
path: Path = typer.Argument(default=Path("."), help="path to project root")
325+
) -> None:
326+
content = json_file.read()
327+
issues = from_json(content, path.absolute())
328+
issues = sorted(issues, key=lambda i: (i.file_name, i.cwe, i.test_id))
329+
print(issues_to_markdown(issues))
330+
331+
260332
def format_jsonl(issue_url: str, issue: Issue) -> str:
261333
issue_json = asdict(issue)
262334
issue_json["issue_url"] = issue_url.strip()

‎poetry.lock‎

Lines changed: 75 additions & 75 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/integration/cli/security-pprint-emty.t‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
Create test input
2+
3+
$ cat > .security.json <<EOF
4+
> {
5+
> "result":[
6+
> ]
7+
> }
8+
> EOF
9+
10+
Run test case
11+
12+
$ tbx security pretty-print .security.json
13+
# Security
14+
15+
|File|line/<br>column|Cwe|Test ID|Details|
16+
|---|:-:|:-:|:-:|---|
17+
18+

‎test/integration/cli/security-pprint.t‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
Create test input
2+
3+
$ cat > .security.json <<EOF
4+
> {
5+
> "results":[
6+
> {
7+
> "code": "555 subprocess.check_call(\n556 config.smv_postbuild_command, cwd=current_cwd, shell=True\n557 )\n558 if config.smv_postbuild_export_pattern != \"\":\n559 matches = find_matching_files_and_dirs(\n",
8+
> "col_offset": 16,
9+
> "end_col_offset": 17,
10+
> "filename": "exasol/toolbox/sphinx/multiversion/main.py",
11+
> "issue_confidence": "HIGH",
12+
> "issue_cwe": {
13+
> "id": 78,
14+
> "link": "https://cwe.mitre.org/data/definitions/78.html"
15+
> },
16+
> "issue_severity": "HIGH",
17+
> "issue_text": "subprocess call with shell=True identified, security issue.",
18+
> "line_number": 556,
19+
> "line_range": [
20+
> 555,
21+
> 556,
22+
> 557
23+
> ],
24+
> "more_info": "https://bandit.readthedocs.io/en/1.7.10/plugins/b602_subprocess_popen_with_shell_equals_true.html",
25+
> "test_id": "B602",
26+
> "test_name": "subprocess_popen_with_shell_equals_true"
27+
> },
28+
> {
29+
> "code": "156 )\n157 subprocess.check_call(cmd, cwd=gitroot, stdout=fp)\n158 fp.seek(0)\n",
30+
> "col_offset": 8,
31+
> "end_col_offset": 58,
32+
> "filename": "exasol/toolbox/sphinx/multiversion/git.py",
33+
> "issue_confidence": "HIGH",
34+
> "issue_cwe": {
35+
> "id": 78,
36+
> "link": "https://cwe.mitre.org/data/definitions/78.html"
37+
> },
38+
> "issue_severity": "LOW",
39+
> "issue_text": "subprocess call - check for execution of untrusted input.",
40+
> "line_number": 157,
41+
> "line_range": [
42+
> 157
43+
> ],
44+
> "more_info": "https://bandit.readthedocs.io/en/1.7.10/plugins/b603_subprocess_without_shell_equals_true.html",
45+
> "test_id": "B603",
46+
> "test_name": "subprocess_without_shell_equals_true"
47+
> },
48+
> {
49+
> "code": "159 with tarfile.TarFile(fileobj=fp) as tarfp:\n160 tarfp.extractall(dst)\n",
50+
> "col_offset": 12,
51+
> "end_col_offset": 33,
52+
> "filename": "exasol/toolbox/sphinx/multiversion/git.py",
53+
> "issue_confidence": "HIGH",
54+
> "issue_cwe": {
55+
> "id": 22,
56+
> "link": "https://cwe.mitre.org/data/definitions/22.html"
57+
> },
58+
> "issue_severity": "HIGH",
59+
> "issue_text": "tarfile.extractall used without any validation. Please check and discard dangerous members.",
60+
> "line_number": 160,
61+
> "line_range": [
62+
> 160
63+
> ],
64+
> "more_info": "https://bandit.readthedocs.io/en/1.7.10/plugins/b202_tarfile_unsafe_members.html",
65+
> "test_id": "B202",
66+
> "test_name": "tarfile_unsafe_members"
67+
> }
68+
> ]
69+
> }
70+
> EOF
71+
72+
Run test case
73+
74+
$ tbx security pretty-print .security.json
75+
# Security
76+
77+
|File|line/<br>column|Cwe|Test ID|Details|
78+
|---|:-:|:-:|:-:|---|
79+
|exasol/toolbox/sphinx/multiversion/git.py|line: 160<br>column: 12|22|B202|https://bandit.readthedocs.io/en/1.7.10/plugins/b202_tarfile_unsafe_members.html ,<br>https://cwe.mitre.org/data/definitions/22.html |
80+
|exasol/toolbox/sphinx/multiversion/git.py|line: 157<br>column: 8|78|B603|https://bandit.readthedocs.io/en/1.7.10/plugins/b603_subprocess_without_shell_equals_true.html ,<br>https://cwe.mitre.org/data/definitions/78.html |
81+
|exasol/toolbox/sphinx/multiversion/main.py|line: 556<br>column: 16|78|B602|https://bandit.readthedocs.io/en/1.7.10/plugins/b602_subprocess_popen_with_shell_equals_true.html ,<br>https://cwe.mitre.org/data/definitions/78.html |

‎test/unit/security_test.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import json
22
import os
3+
import pathlib
34
import subprocess
45
from contextlib import contextmanager
56
from inspect import cleandoc
@@ -403,3 +404,61 @@ def test_format_jsonl_removes_newline():
403404
)
404405
actual = security.format_jsonl("my_issue_url\n", issue)
405406
assert actual == expected
407+
408+
409+
@pytest.mark.parametrize(
410+
"json_file,expected",
411+
[
412+
(
413+
'''{
414+
"results": [
415+
{
416+
"code": "1 import subprocess\\n2 from typing import Iterable\\n3 \\n",
417+
"col_offset": 12,
418+
"end_col_offset": 17,
419+
"filename": "/home/test/python-toolbox/exasol/toolbox/git.py",
420+
"issue_confidence": "HIGH",
421+
"issue_cwe": {
422+
"id": 78,
423+
"link": "https://cwe.mitre.org/data/definitions/78.html"
424+
},
425+
"issue_severity": "LOW",
426+
"issue_text": "Consider possible security implications associated with the subprocess module.",
427+
"line_number": 53,
428+
"line_range": [
429+
1
430+
],
431+
"more_info": "https://bandit.readthedocs.io/en/1.7.10/blacklists/blacklist_imports.html#b404-import-subprocess",
432+
"test_id": "B404",
433+
"test_name": "blacklist"
434+
}
435+
]
436+
}
437+
''',
438+
{
439+
"file_name": "exasol/toolbox/git.py",
440+
"line": 53,
441+
"column": 12,
442+
"cwe": "78",
443+
"test_id": "B404",
444+
"description": "Consider possible security implications associated with the subprocess module.",
445+
"references": (
446+
"https://bandit.readthedocs.io/en/1.7.10/blacklists/blacklist_imports.html#b404-import-subprocess",
447+
"https://cwe.mitre.org/data/definitions/78.html"
448+
)
449+
}
450+
)
451+
]
452+
)
453+
def test_from_json(json_file, expected):
454+
actual = security.from_json(json_file, pathlib.Path("/home/test/python-toolbox"))
455+
expected_issue = security.SecurityIssue(
456+
file_name=expected["file_name"],
457+
line=expected["line"],
458+
column=expected["column"],
459+
cwe=expected["cwe"],
460+
test_id=expected["test_id"],
461+
description=expected["description"],
462+
references=expected["references"]
463+
)
464+
assert list(actual) == [expected_issue]

0 commit comments

Comments
 (0)