Skip to content

Commit

Permalink
Add tests for SAST and Review comments (jfrog#523)
Browse files Browse the repository at this point in the history
  • Loading branch information
@attiasas
attiasas authored Oct 8, 2023
1 parent 2504605 commit 47d00dd
Show file tree
Hide file tree
Showing 10 changed files with 898 additions and 173 deletions.
355 changes: 246 additions & 109 deletions scanpullrequest/scanpullrequest_test.go
View file

Large diffs are not rendered by default.

10 changes: 10 additions & 0 deletions utils/outputwriter/outputwriter.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,14 @@ import (
const (
FrogbotTitlePrefix = "[🐸 Frogbot]"
CommentGeneratedByFrogbot = "[🐸 JFrog Frogbot](https://github.com/jfrog/frogbot#readme)"
ReviewCommentId = "FrogbotReviewComment"
vulnerabilitiesTableHeader = "\n| SEVERITY | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS | CVES |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | :---------------------------------: |"
vulnerabilitiesTableHeaderWithContextualAnalysis = "| SEVERITY | CONTEXTUAL ANALYSIS | DIRECT DEPENDENCIES | IMPACTED DEPENDENCY | FIXED VERSIONS | CVES |\n| :---------------------: | :----------------------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: | :---------------------------------: |"
iacTableHeader = "\n| SEVERITY | FILE | LINE:COLUMN | FINDING |\n| :---------------------: | :----------------------------------: | :-----------------------------------: | :---------------------------------: |"
vulnerableDependenciesTitle = "## 📦 Vulnerable Dependencies"
summaryTitle = "### ✍️ Summary"
researchDetailsTitle = "## 🔬 Research Details"
sastTitle = "## 🎯 Static Application Security Testing (SAST) Vulnerability"
iacTitle = "## 🛠️ Infrastructure as Code"
licenseTitle = "## ⚖️ Violated Licenses"
contextualAnalysisTitle = "## 📦🔍 Contextual Analysis CVE Vulnerability\n"
Expand Down Expand Up @@ -254,3 +256,11 @@ func getVulnerabilityDescriptionIdentifier(cveRows []formats.CveRow, xrayId stri
}
return fmt.Sprintf("[ %s ] ", identifier)
}

func GenerateReviewCommentContent(content string, writer OutputWriter) string {
return MarkdownComment(ReviewCommentId) + content + writer.Footer()
}

func GetFallbackReviewCommentContent(content string, location formats.Location, writer OutputWriter) string {
return MarkdownComment(ReviewCommentId) + GetLocationDescription(location) + content + writer.Footer()
}
115 changes: 115 additions & 0 deletions utils/outputwriter/outputwriter_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,3 +62,118 @@ func testGetLicensesTableContent(t *testing.T, writer OutputWriter) {
expected = "\n| License1 | Comp1 1.0 | Dep1 2.0 |\n| License2 | Comp2 2.0 | Dep2 3.0 |"
assert.Equal(t, expected, result)
}

func TestMarkAsQuote(t *testing.T) {
testCases := []struct {
input string
expectedOutput string
}{
{
input: "",
expectedOutput: "``",
},
{
input: "quote",
expectedOutput: "`quote`",
},
}
for _, tc := range testCases {
assert.Equal(t, tc.expectedOutput, MarkAsQuote(tc.input))
}
}

func TestMarkAsCodeSnippet(t *testing.T) {
testCases := []struct {
input string
expectedOutput string
}{
{
input: "",
expectedOutput: "```\n\n```",
},
{
input: "snippet",
expectedOutput: "```\nsnippet\n```",
},
}
for _, tc := range testCases {
assert.Equal(t, tc.expectedOutput, MarkAsCodeSnippet(tc.input))
}
}

func TestGetLocationDescription(t *testing.T) {
testCases := []struct {
input formats.Location
expectedOutput string
}{
{
input: formats.Location{
File: "file1",
StartLine: 1,
Snippet: "snippet",
},
expectedOutput: "\n```\nsnippet\n```\nat `file1` (line 1)\n",
},
{
input: formats.Location{
File: "dir/other-dir/file1",
StartLine: 134,
Snippet: "clientTestUtils.ChangeDirAndAssert(t, prevWd)",
},
expectedOutput: "\n```\nclientTestUtils.ChangeDirAndAssert(t, prevWd)\n```\nat `dir/other-dir/file1` (line 134)\n",
},
}
for _, tc := range testCases {
assert.Equal(t, tc.expectedOutput, GetLocationDescription(tc.input))
}
}

func TestGetJasMarkdownDescription(t *testing.T) {
testCases := []struct {
severity string
finding string
expectedOutput string
}{
{
severity: "High",
finding: "finding",
expectedOutput: "| Severity | Finding |\n| :--------------: | :---: |\n| High | finding |",
},
{
severity: "Low",
finding: "finding (other)",
expectedOutput: "| Severity | Finding |\n| :--------------: | :---: |\n| Low | finding (other) |",
},
}
for _, tc := range testCases {
assert.Equal(t, tc.expectedOutput, GetJasMarkdownDescription(tc.severity, tc.finding))
}
}

func TestGetApplicabilityMarkdownDescription(t *testing.T) {
testCases := []struct {
severity string
cve string
impactedDependency string
finding string
expectedOutput string
}{
{
severity: "High",
cve: "CVE-100-234",
impactedDependency: "dependency:1.0.0",
finding: "applicable finding",
expectedOutput: "| Severity | Impacted Dependency | Finding | CVE |\n| :--------------: | :---: | :---: | :---: |\n| High | dependency:1.0.0 | applicable finding | CVE-100-234 |",
},
{
severity: "Low",
cve: "CVE-222-233",
impactedDependency: "dependency:3.4.1",
finding: "applicable finding (diff)",
expectedOutput: "| Severity | Impacted Dependency | Finding | CVE |\n| :--------------: | :---: | :---: | :---: |\n| Low | dependency:3.4.1 | applicable finding (diff) | CVE-222-233 |",
},
}
for _, tc := range testCases {
assert.Equal(t, tc.expectedOutput, GetApplicabilityMarkdownDescription(tc.severity, tc.cve, tc.impactedDependency, tc.finding))
}
}
8 changes: 4 additions & 4 deletions utils/outputwriter/simplifiedoutput.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -191,16 +191,16 @@ func (smo *SimplifiedOutput) SastReviewContent(severity, finding, fullDetails st
### Full description
%s
---
### Code Flows
`,
GetJasMarkdownDescription(smo.FormattedSeverity(severity, "Applicable"), finding),
fullDetails,
))

if len(codeFlows) > 0 {
contentBuilder.WriteString(`
---
### Code Flows
`)
for _, flow := range codeFlows {
contentBuilder.WriteString(`
Expand Down
72 changes: 72 additions & 0 deletions utils/outputwriter/simplifiedoutput_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -375,3 +375,75 @@ func TestSimplifiedOutput_GetLicensesTableContent(t *testing.T) {
writer := &SimplifiedOutput{}
testGetLicensesTableContent(t, writer)
}

func TestSimplifiedOutput_SastReviewContent(t *testing.T) {
testCases := []struct {
name string
severity string
finding string
fullDetails string
expectedOutput string
codeFlows [][]formats.Location
}{
{
name: "Sast review comment content",
severity: "Low",
finding: "Stack Trace Exposure",
fullDetails: "\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.",
codeFlows: [][]formats.Location{
{
{
File: "file2",
StartLine: 1,
StartColumn: 2,
EndLine: 3,
EndColumn: 4,
Snippet: "other-snippet",
},
{
File: "file",
StartLine: 0,
StartColumn: 0,
EndLine: 0,
EndColumn: 0,
Snippet: "snippet",
},
},
{
{
File: "file",
StartLine: 10,
StartColumn: 20,
EndLine: 10,
EndColumn: 30,
Snippet: "a-snippet",
},
{
File: "file",
StartLine: 0,
StartColumn: 0,
EndLine: 0,
EndColumn: 0,
Snippet: "snippet",
},
},
},
expectedOutput: "\n## 🎯 Static Application Security Testing (SAST) Vulnerability\n\t\n| Severity | Finding |\n| :--------------: | :---: |\n| Low | Stack Trace Exposure |\n\n---\n### Full description\n\n\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.\n\n---\n### Code Flows\n\n\n---\nVulnerable data flow analysis result:\n\n↘️ `other-snippet` (at file2 line 1)\n\n↘️ `snippet` (at file line 0)\n\n\n---\n\n\n\n---\nVulnerable data flow analysis result:\n\n↘️ `a-snippet` (at file line 10)\n\n↘️ `snippet` (at file line 0)\n\n\n---\n\n",
},
{
name: "No code flows",
severity: "Low",
finding: "Stack Trace Exposure",
fullDetails: "\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.",
expectedOutput: "\n## 🎯 Static Application Security Testing (SAST) Vulnerability\n\t\n| Severity | Finding |\n| :--------------: | :---: |\n| Low | Stack Trace Exposure |\n\n---\n### Full description\n\n\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.\n",
},
}

so := &SimplifiedOutput{}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
output := so.SastReviewContent(tc.severity, tc.finding, tc.fullDetails, tc.codeFlows)
assert.Equal(t, tc.expectedOutput, output)
})
}
}
134 changes: 134 additions & 0 deletions utils/outputwriter/standardoutput_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -403,3 +403,137 @@ func TestStandardOutput_GetLicensesTableContent(t *testing.T) {
writer := &StandardOutput{}
testGetLicensesTableContent(t, writer)
}

func TestStandardOutput_ApplicableCveReviewContent(t *testing.T) {
testCases := []struct {
name string
severity, finding, fullDetails, cve, cveDetails, impactedDependency, remediation string
expectedOutput string
}{
{
name: "Applicable CVE review comment content",
severity: "Critical",
finding: "The vulnerable function flask.Flask.run is called",
fullDetails: "The scanner checks whether the vulnerable `Development Server` of the `werkzeug` library is used by looking for calls to `werkzeug.serving.run_simple()`.",
cve: "CVE-2022-29361",
cveDetails: "cveDetails",
impactedDependency: "werkzeug:1.0.1",
remediation: "some remediation",
expectedOutput: "\n## 📦🔍 Contextual Analysis CVE Vulnerability\n\n\n<div align=\"center\">\n\n| Severity | Impacted Dependency | Finding | CVE |\n| :--------------: | :---: | :---: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableCriticalSeverity.png)<br>Critical | werkzeug:1.0.1 | The vulnerable function flask.Flask.run is called | CVE-2022-29361 |\n\n</div>\n\n<details>\n<summary> <b>Description</b> </summary>\n<br>\n\nThe scanner checks whether the vulnerable `Development Server` of the `werkzeug` library is used by looking for calls to `werkzeug.serving.run_simple()`.\n\n</details>\n\n<details>\n<summary> <b>CVE details</b> </summary>\n<br>\n\ncveDetails\n\n</details>\n\n\n<details>\n<summary> <b>Remediation</b> </summary>\n<br>\n\nsome remediation\n\n</details>\n\n",
},
{
name: "No remediation",
severity: "Critical",
finding: "The vulnerable function flask.Flask.run is called",
fullDetails: "The scanner checks whether the vulnerable `Development Server` of the `werkzeug` library is used by looking for calls to `werkzeug.serving.run_simple()`.",
cve: "CVE-2022-29361",
cveDetails: "cveDetails",
impactedDependency: "werkzeug:1.0.1",
expectedOutput: "\n## 📦🔍 Contextual Analysis CVE Vulnerability\n\n\n<div align=\"center\">\n\n| Severity | Impacted Dependency | Finding | CVE |\n| :--------------: | :---: | :---: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableCriticalSeverity.png)<br>Critical | werkzeug:1.0.1 | The vulnerable function flask.Flask.run is called | CVE-2022-29361 |\n\n</div>\n\n<details>\n<summary> <b>Description</b> </summary>\n<br>\n\nThe scanner checks whether the vulnerable `Development Server` of the `werkzeug` library is used by looking for calls to `werkzeug.serving.run_simple()`.\n\n</details>\n\n<details>\n<summary> <b>CVE details</b> </summary>\n<br>\n\ncveDetails\n\n</details>\n\n",
},
}

so := &StandardOutput{}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
output := so.ApplicableCveReviewContent(tc.severity, tc.finding, tc.fullDetails, tc.cve, tc.cveDetails, tc.impactedDependency, tc.remediation)
assert.Equal(t, tc.expectedOutput, output)
})
}
}

func TestStandardOutput_IacReviewContent(t *testing.T) {
testCases := []struct {
name string
severity, finding, fullDetails string
expectedOutput string
}{
{
name: "Iac review comment content",
severity: "Medium",
finding: "Missing auto upgrade was detected",
fullDetails: "Resource `google_container_node_pool` should have `management.auto_upgrade=true`\n\nVulnerable example - \n```\nresource \"google_container_node_pool\" \"vulnerable_example\" {\n management {\n auto_upgrade = false\n }\n}\n```\n",
expectedOutput: "\n## 🛠️ Infrastructure as Code\n\n<div align=\"center\">\n\n| Severity | Finding |\n| :--------------: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableMediumSeverity.png)<br> Medium | Missing auto upgrade was detected |\n\n</div>\n\n<details>\n<summary> <b>Full description</b> </summary>\n<br>\n\nResource `google_container_node_pool` should have `management.auto_upgrade=true`\n\nVulnerable example - \n```\nresource \"google_container_node_pool\" \"vulnerable_example\" {\n management {\n auto_upgrade = false\n }\n}\n```\n\n\n</details>\n\n",
},
}

so := &StandardOutput{}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
output := so.IacReviewContent(tc.severity, tc.finding, tc.fullDetails)
assert.Equal(t, tc.expectedOutput, output)
})
}
}

func TestStandardOutput_SastReviewContent(t *testing.T) {
testCases := []struct {
name string
severity string
finding string
fullDetails string
expectedOutput string
codeFlows [][]formats.Location
}{
{
name: "Sast review comment content",
severity: "Low",
finding: "Stack Trace Exposure",
fullDetails: "\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.",
codeFlows: [][]formats.Location{
{
{
File: "file2",
StartLine: 1,
StartColumn: 2,
EndLine: 3,
EndColumn: 4,
Snippet: "other-snippet",
},
{
File: "file",
StartLine: 0,
StartColumn: 0,
EndLine: 0,
EndColumn: 0,
Snippet: "snippet",
},
},
{
{
File: "file",
StartLine: 10,
StartColumn: 20,
EndLine: 10,
EndColumn: 30,
Snippet: "a-snippet",
},
{
File: "file",
StartLine: 0,
StartColumn: 0,
EndLine: 0,
EndColumn: 0,
Snippet: "snippet",
},
},
},
expectedOutput: "\n## 🎯 Static Application Security Testing (SAST) Vulnerability \n\t\n<div align=\"center\">\n\n| Severity | Finding |\n| :--------------: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableLowSeverity.png)<br> Low | Stack Trace Exposure |\n\n</div>\n\n<details>\n<summary> <b>Full description</b> </summary>\n<br>\n\n\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.\n\n</details>\n\n\n\n<details>\n<summary><b>Code Flows</b> </summary>\n\n\n\n<details>\n<summary><b>Vulnerable data flow analysis result</b> </summary>\n<br>\n\n↘️ `other-snippet` (at file2 line 1)\n\n↘️ `snippet` (at file line 0)\n\n\n</details>\n\n\n\n<details>\n<summary><b>Vulnerable data flow analysis result</b> </summary>\n<br>\n\n↘️ `a-snippet` (at file line 10)\n\n↘️ `snippet` (at file line 0)\n\n\n</details>\n\n\n\n</details>\n\n",
},
{
name: "No code flows",
severity: "Low",
finding: "Stack Trace Exposure",
fullDetails: "\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.",
expectedOutput: "\n## 🎯 Static Application Security Testing (SAST) Vulnerability \n\t\n<div align=\"center\">\n\n| Severity | Finding |\n| :--------------: | :---: |\n| ![](https://raw.githubusercontent.com/jfrog/frogbot/master/resources/v2/applicableLowSeverity.png)<br> Low | Stack Trace Exposure |\n\n</div>\n\n<details>\n<summary> <b>Full description</b> </summary>\n<br>\n\n\n### Overview\nStack trace exposure is a type of security vulnerability that occurs when a program reveals\nsensitive information, such as the names and locations of internal files and variables,\nin error messages or other diagnostic output. This can happen when a program crashes or\nencounters an error, and the stack trace (a record of the program's call stack at the time\nof the error) is included in the output.\n\n</details>\n\n",
},
}

so := &StandardOutput{}
for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
output := so.SastReviewContent(tc.severity, tc.finding, tc.fullDetails, tc.codeFlows)
assert.Equal(t, tc.expectedOutput, output)
})
}
}
Loading

0 comments on commit 47d00dd

Please sign in to comment.