Skip to content

Commit bb72551

Browse files
attiasasattiasas
authored
Fix bugs when converting results to SARIF (jfrog#234)
1 parent a706636 commit bb72551

File tree

7 files changed

+120
-39
lines changed

7 files changed

+120
-39
lines changed

‎tests/testdata/output/audit/audit_sarif.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1106,7 +1106,7 @@
11061106
"ruleIndex": 6,
11071107
"level": "warning",
11081108
"message": {
1109-
"text": "[CVE-2020-28500] lodash 4.17.0"
1109+
"text": "Security violation [CVE-2020-28500] lodash 4.17.0"
11101110
},
11111111
"locations": [
11121112
{

‎tests/testdata/output/dockerscan/docker_results.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -616,7 +616,7 @@
616616
],
617617
"executionSuccessful": true,
618618
"workingDirectory": {
619-
"uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017/image.tar"
619+
"uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017"
620620
}
621621
}
622622
],
@@ -815,7 +815,7 @@
815815
],
816816
"executionSuccessful": true,
817817
"workingDirectory": {
818-
"uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017/image.tar"
818+
"uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017"
819819
}
820820
}
821821
],

‎tests/testdata/output/dockerscan/docker_sarif.json‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -119,7 +119,7 @@
119119
],
120120
"executionSuccessful": true,
121121
"workingDirectory": {
122-
"uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017/image.tar"
122+
"uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017"
123123
}
124124
}
125125
],
@@ -393,7 +393,7 @@
393393
{
394394
"executionSuccessful": true,
395395
"workingDirectory": {
396-
"uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017/image.tar"
396+
"uri": "/var/folders/xv/th4cksxn7jv9wjrdnn1h4tj00000gq/T/jfrog.cli.temp.-1726210535-1985298017"
397397
}
398398
}
399399
],

‎utils/formats/sarifutils/sarifutils.go‎

Lines changed: 72 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,17 @@ func GetToolVersion(run *sarif.Run) string {
3737
return ""
3838
}
3939

40+
func CopyRun(run *sarif.Run) *sarif.Run {
41+
copy := CopyRunMetadata(run)
42+
if copy.Tool.Driver != nil {
43+
copy.Tool.Driver.Rules = CopyRules(run.Tool.Driver.Rules...)
44+
}
45+
for _, result := range run.Results {
46+
copy.Results = append(copy.Results, CopyResult(result))
47+
}
48+
return copy
49+
}
50+
4051
func CopyRunMetadata(run *sarif.Run) (copied *sarif.Run) {
4152
if run == nil {
4253
return
@@ -64,6 +75,21 @@ func CopyRunMetadata(run *sarif.Run) (copied *sarif.Run) {
6475
return
6576
}
6677

78+
func CopyRules(rules ...*sarif.ReportingDescriptor) (copied []*sarif.ReportingDescriptor) {
79+
for _, rule := range rules {
80+
cloned := sarif.NewRule(rule.ID)
81+
cloned.HelpURI = copyStrAttribute(rule.HelpURI)
82+
cloned.Name = copyStrAttribute(rule.Name)
83+
cloned.ShortDescription = copyMultiMsgAttribute(rule.ShortDescription)
84+
cloned.FullDescription = copyMultiMsgAttribute(rule.FullDescription)
85+
cloned.Help = copyMultiMsgAttribute(rule.Help)
86+
cloned.Properties = rule.Properties
87+
cloned.MessageStrings = rule.MessageStrings
88+
copied = append(copied, cloned)
89+
}
90+
return
91+
}
92+
6793
func GetRunToolFullName(run *sarif.Run) string {
6894
if run.Tool.Driver != nil && run.Tool.Driver.FullName != nil {
6995
return *run.Tool.Driver.FullName
@@ -134,9 +160,9 @@ func CopyResult(result *sarif.Result) *sarif.Result {
134160
RuleIndex: result.RuleIndex,
135161
Kind: result.Kind,
136162
Fingerprints: result.Fingerprints,
137-
CodeFlows: result.CodeFlows,
163+
CodeFlows: copyCodeFlows(result.CodeFlows...),
138164
Level: result.Level,
139-
Message: result.Message,
165+
Message: copyMsgAttribute(result.Message),
140166
PropertyBag: result.PropertyBag,
141167
}
142168
for _, location := range result.Locations {
@@ -145,6 +171,47 @@ func CopyResult(result *sarif.Result) *sarif.Result {
145171
return copied
146172
}
147173

174+
func copyCodeFlows(flows ...*sarif.CodeFlow) []*sarif.CodeFlow {
175+
var copied []*sarif.CodeFlow
176+
for _, flow := range flows {
177+
copied = append(copied, copyCodeFlow(flow))
178+
}
179+
return copied
180+
}
181+
182+
func copyCodeFlow(flow *sarif.CodeFlow) *sarif.CodeFlow {
183+
copied := &sarif.CodeFlow{}
184+
for _, threadFlow := range flow.ThreadFlows {
185+
copied.ThreadFlows = append(copied.ThreadFlows, copyThreadFlow(threadFlow))
186+
}
187+
return copied
188+
}
189+
190+
func copyThreadFlow(threadFlow *sarif.ThreadFlow) *sarif.ThreadFlow {
191+
copied := &sarif.ThreadFlow{}
192+
for _, location := range threadFlow.Locations {
193+
copied.Locations = append(copied.Locations, sarif.NewThreadFlowLocation().WithLocation(CopyLocation(location.Location)))
194+
}
195+
return copied
196+
}
197+
198+
func copyMsgAttribute(attr sarif.Message) sarif.Message {
199+
return sarif.Message{
200+
Text: copyStrAttribute(attr.Text),
201+
Markdown: copyStrAttribute(attr.Markdown),
202+
}
203+
}
204+
205+
func copyMultiMsgAttribute(attr *sarif.MultiformatMessageString) *sarif.MultiformatMessageString {
206+
if attr == nil {
207+
return nil
208+
}
209+
return &sarif.MultiformatMessageString{
210+
Text: copyStrAttribute(attr.Text),
211+
Markdown: copyStrAttribute(attr.Markdown),
212+
}
213+
}
214+
148215
func copyStrAttribute(attr *string) *string {
149216
if attr == nil {
150217
return nil
@@ -190,9 +257,9 @@ func CopyLocation(location *sarif.Location) *sarif.Location {
190257
copied.Properties = location.Properties
191258
for _, logicalLocation := range location.LogicalLocations {
192259
copied.LogicalLocations = append(copied.LogicalLocations, &sarif.LogicalLocation{
193-
Name: logicalLocation.Name,
194-
FullyQualifiedName: logicalLocation.FullyQualifiedName,
195-
DecoratedName: logicalLocation.DecoratedName,
260+
Name: copyStrAttribute(logicalLocation.Name),
261+
FullyQualifiedName: copyStrAttribute(logicalLocation.FullyQualifiedName),
262+
DecoratedName: copyStrAttribute(logicalLocation.DecoratedName),
196263
Kind: logicalLocation.Kind,
197264
PropertyBag: logicalLocation.PropertyBag,
198265
})

‎utils/results/conversion/sarifparser/sarifparser.go‎

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -110,8 +110,13 @@ func (sc *CmdResultsSarifConverter) ParseNewTargetResults(target results.ScanTar
110110
func (sc *CmdResultsSarifConverter) createScaRun(target results.ScanTarget, errorCount int) *sarif.Run {
111111
run := sarif.NewRunWithInformationURI(ScaScannerToolName, utils.BaseDocumentationURL+"sca")
112112
run.Tool.Driver.Version = &sc.xrayVersion
113+
wd := target.Target
114+
if sc.currentCmdType.IsTargetBinary() {
115+
// For binary, the target is a file and not a directory
116+
wd = filepath.Dir(wd)
117+
}
113118
run.Invocations = append(run.Invocations, sarif.NewInvocation().
114-
WithWorkingDirectory(sarif.NewSimpleArtifactLocation(target.Target)).
119+
WithWorkingDirectory(sarif.NewSimpleArtifactLocation(wd)).
115120
WithExecutionSuccess(errorCount == 0),
116121
)
117122
return run
@@ -240,7 +245,7 @@ func addSarifScaVulnerability(cmdType utils.CommandType, sarifResults *[]*sarif.
240245
if err != nil {
241246
return err
242247
}
243-
currentResults, currentRule := parseScaToSarifFormat(cmdType, vulnerability.IssueId, vulnerability.Summary, markdownDescription, maxCveScore, getScaIssueSarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents)
248+
currentResults, currentRule := parseScaToSarifFormat(cmdType, vulnerability.IssueId, vulnerability.Summary, markdownDescription, maxCveScore, getScaVulnerabilitySarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents)
244249
cveImpactedComponentRuleId := results.GetScaIssueId(impactedPackagesName, impactedPackagesVersion, results.GetIssueIdentifier(cves, vulnerability.IssueId, "_"))
245250
if _, ok := (*rules)[cveImpactedComponentRuleId]; !ok {
246251
// New Rule
@@ -261,7 +266,7 @@ func addSarifScaSecurityViolation(cmdType utils.CommandType, sarifResults *[]*sa
261266
if err != nil {
262267
return err
263268
}
264-
currentResults, currentRule := parseScaToSarifFormat(cmdType, violation.IssueId, violation.Summary, markdownDescription, maxCveScore, getScaIssueSarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents, violation.WatchName)
269+
currentResults, currentRule := parseScaToSarifFormat(cmdType, violation.IssueId, violation.Summary, markdownDescription, maxCveScore, getScaSecurityViolationSarifHeadline, cves, severity, applicabilityStatus, impactedPackagesName, impactedPackagesVersion, fixedVersions, directComponents, violation.WatchName)
265270
cveImpactedComponentRuleId := results.GetScaIssueId(impactedPackagesName, impactedPackagesVersion, results.GetIssueIdentifier(cves, violation.IssueId, "_"))
266271
if _, ok := (*rules)[cveImpactedComponentRuleId]; !ok {
267272
// New Rule
@@ -396,10 +401,14 @@ func getDirectDependenciesFormatted(directDependencies []formats.ComponentRow) (
396401
return strings.TrimSuffix(formattedDirectDependencies.String(), "<br/>"), nil
397402
}
398403

399-
func getScaIssueSarifHeadline(depName, version, issueId string) string {
404+
func getScaVulnerabilitySarifHeadline(depName, version, issueId string) string {
400405
return fmt.Sprintf("[%s] %s %s", issueId, depName, version)
401406
}
402407

408+
func getScaSecurityViolationSarifHeadline(depName, version, key string) string {
409+
return fmt.Sprintf("Security violation %s", getScaVulnerabilitySarifHeadline(depName, version, key))
410+
}
411+
403412
func getXrayLicenseSarifHeadline(depName, version, key string) string {
404413
return fmt.Sprintf("License violation [%s] in %s %s", key, depName, version)
405414
}
@@ -417,21 +426,21 @@ func getScaLicenseViolationMarkdown(depName, version, key string, directDependen
417426
}
418427

419428
func patchRunsToPassIngestionRules(cmdType utils.CommandType, subScanType utils.SubScanType, patchBinaryPaths bool, target results.ScanTarget, runs ...*sarif.Run) []*sarif.Run {
420-
// Since we run in temp directories files should be relative
421-
// Patch by converting the file paths to relative paths according to the invocations
422-
convertPaths(cmdType, subScanType, runs...)
423429
patchedRuns := []*sarif.Run{}
424430
// Patch changes may alter the original run, so we will create a new run for each
425431
for _, run := range runs {
426-
patched := sarifutils.CopyRunMetadata(run)
432+
patched := sarifutils.CopyRun(run)
433+
// Since we run in temp directories files should be relative
434+
// Patch by converting the file paths to relative paths according to the invocations
435+
convertPaths(cmdType, subScanType, patched)
427436
if cmdType.IsTargetBinary() && subScanType == utils.SecretsScan {
428437
// Patch the tool name in case of binary scan
429438
sarifutils.SetRunToolName(BinarySecretScannerToolName, patched)
430439
}
431440
if patched.Tool.Driver != nil {
432-
patched.Tool.Driver.Rules = patchRules(cmdType, subScanType, run.Tool.Driver.Rules...)
441+
patched.Tool.Driver.Rules = patchRules(cmdType, subScanType, patched.Tool.Driver.Rules...)
433442
}
434-
patched.Results = patchResults(cmdType, subScanType, patchBinaryPaths, target, run, run.Results...)
443+
patched.Results = patchResults(cmdType, subScanType, patchBinaryPaths, target, patched, patched.Results...)
435444
patchedRuns = append(patchedRuns, patched)
436445
}
437446
return patchedRuns
@@ -470,28 +479,20 @@ func patchDockerSecretLocations(result *sarif.Result) {
470479
func patchRules(commandType utils.CommandType, subScanType utils.SubScanType, rules ...*sarif.ReportingDescriptor) (patched []*sarif.ReportingDescriptor) {
471480
patched = []*sarif.ReportingDescriptor{}
472481
for _, rule := range rules {
473-
cloned := sarif.NewRule(rule.ID)
474482
if rule.Name != nil && rule.ID == *rule.Name {
475483
// SARIF1001 - if both 'id' and 'name' are present, they must be different. If they are identical, the tool must omit the 'name' property.
476-
cloned.Name = rule.Name
484+
rule.Name = nil
477485
}
478-
cloned.ShortDescription = rule.ShortDescription
479486
if commandType.IsTargetBinary() && subScanType == utils.SecretsScan {
480487
// Patch the rule name in case of binary scan
481-
sarifutils.SetRuleShortDescriptionText(fmt.Sprintf("[Secret in Binary found] %s", sarifutils.GetRuleShortDescriptionText(rule)), cloned)
488+
sarifutils.SetRuleShortDescriptionText(fmt.Sprintf("[Secret in Binary found] %s", sarifutils.GetRuleShortDescriptionText(rule)), rule)
482489
}
483-
cloned.FullDescription = rule.FullDescription
484-
cloned.Help = rule.Help
485-
if cloned.Help == nil {
490+
if rule.Help == nil {
486491
// Github code scanning ingestion rules rejects rules without help content.
487492
// Patch by transferring the full description to the help field.
488-
cloned.Help = rule.FullDescription
493+
rule.Help = rule.FullDescription
489494
}
490-
cloned.HelpURI = rule.HelpURI
491-
cloned.Properties = rule.Properties
492-
cloned.MessageStrings = rule.MessageStrings
493-
494-
patched = append(patched, cloned)
495+
patched = append(patched, rule)
495496
}
496497
return
497498
}
@@ -734,7 +735,7 @@ func calculateResultFingerprints(resultType utils.CommandType, run *sarif.Run, r
734735
if !resultType.IsTargetBinary() {
735736
return nil
736737
}
737-
ids := []string{sarifutils.GetRunToolName(run), sarifutils.GetResultRuleId(result)}
738+
ids := []string{sarifutils.GetRunToolName(run), sarifutils.GetResultRuleId(result), getResultWatches(result)}
738739
for _, location := range sarifutils.GetResultFileLocations(result) {
739740
ids = append(ids, strings.ReplaceAll(location, string(filepath.Separator), "/"))
740741
}
@@ -747,3 +748,12 @@ func calculateResultFingerprints(resultType utils.CommandType, run *sarif.Run, r
747748
sarifutils.SetResultFingerprint(jfrogFingerprintAlgorithmName, hashValue, result)
748749
return nil
749750
}
751+
752+
func getResultWatches(result *sarif.Result) (watches string) {
753+
if watchesProperty, ok := result.Properties[WatchSarifPropertyKey]; ok {
754+
if watchesValue, ok := watchesProperty.(string); ok {
755+
return watchesValue
756+
}
757+
}
758+
return
759+
}

‎utils/results/conversion/sarifparser/sarifparser_test.go‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -65,8 +65,13 @@ func TestGetComponentSarifLocation(t *testing.T) {
6565
}
6666

6767
func TestGetVulnerabilityOrViolationSarifHeadline(t *testing.T) {
68-
assert.Equal(t, "[CVE-2022-1234] loadsh 1.4.1", getScaIssueSarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
69-
assert.NotEqual(t, "[CVE-2022-1234] comp 1.4.1", getScaIssueSarifHeadline("comp", "1.2.1", "CVE-2022-1234"))
68+
assert.Equal(t, "[CVE-2022-1234] loadsh 1.4.1", getScaVulnerabilitySarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
69+
assert.NotEqual(t, "[CVE-2022-1234] comp 1.4.1", getScaVulnerabilitySarifHeadline("comp", "1.2.1", "CVE-2022-1234"))
70+
}
71+
72+
func TestGetScaSecurityViolationSarifHeadline(t *testing.T) {
73+
assert.Equal(t, "Security violation [CVE-2022-1234] loadsh 1.4.1", getScaSecurityViolationSarifHeadline("loadsh", "1.4.1", "CVE-2022-1234"))
74+
assert.NotEqual(t, "[CVE-2022-1234] comp 1.2.1", getScaSecurityViolationSarifHeadline("comp", "1.2.1", "CVE-2022-1234"))
7075
}
7176

7277
func TestGetXrayLicenseSarifHeadline(t *testing.T) {
@@ -411,7 +416,7 @@ func TestPatchRunsToPassIngestionRules(t *testing.T) {
411416
},
412417
Invocations: []*sarif.Invocation{sarif.NewInvocation().WithWorkingDirectory(sarif.NewSimpleArtifactLocation(wd))},
413418
Results: []*sarif.Result{
414-
sarifutils.CreateDummyResultWithFingerprint(fmt.Sprintf("🔒 Found Secrets in Binary docker scanning:\nImage: dockerImage:imageVersion\nLayer (sha1): 9e88ea9de1b44baba5e96a79e33e4af64334b2bf129e838e12f6dae71b5c86f0\nFilepath: %s\nEvidence: snippet", filepath.Join("usr", "src", "app", "server", "index.js")), "", jfrogFingerprintAlgorithmName, "93d660ebfd39b1220c42c0beb6e4e863",
419+
sarifutils.CreateDummyResultWithFingerprint(fmt.Sprintf("🔒 Found Secrets in Binary docker scanning:\nImage: dockerImage:imageVersion\nLayer (sha1): 9e88ea9de1b44baba5e96a79e33e4af64334b2bf129e838e12f6dae71b5c86f0\nFilepath: %s\nEvidence: snippet", filepath.Join("usr", "src", "app", "server", "index.js")), "", jfrogFingerprintAlgorithmName, "dee156c9fd75a4237102dc8fb29277a2",
415420
sarifutils.CreateDummyLocationWithPathAndLogicalLocation(filepath.Join("usr", "src", "app", "server", "index.js"), "9e88ea9de1b44baba5e96a79e33e4af64334b2bf129e838e12f6dae71b5c86f0", "layer", "algorithm", "sha1"),
416421
),
417422
},

‎utils/validations/test_validate_sarif.go‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,6 @@ func getResultByResultId(expected *sarif.Result, actual []*sarif.Result) *sarif.
191191
log.Output("====================================")
192192
log.Output(fmt.Sprintf(":: Actual results with expected results: %s", getResultId(expected)))
193193
for _, result := range actual {
194-
195194
log.Output(fmt.Sprintf("Compare actual result (isPotential=%t, hasSameLocations=%t) with expected result: %s", isPotentialSimilarResults(expected, result), hasSameLocations(expected, result), getResultId(result)))
196195
if isPotentialSimilarResults(expected, result) && hasSameLocations(expected, result) {
197196
return result
@@ -202,7 +201,7 @@ func getResultByResultId(expected *sarif.Result, actual []*sarif.Result) *sarif.
202201
}
203202

204203
func isPotentialSimilarResults(expected, actual *sarif.Result) bool {
205-
return sarifutils.GetResultRuleId(actual) == sarifutils.GetResultRuleId(expected) && sarifutils.GetResultMsgText(actual) == sarifutils.GetResultMsgText(expected) && sarifutils.GetResultProperty(sarifparser.WatchSarifPropertyKey, actual) == sarifutils.GetResultProperty(sarifparser.WatchSarifPropertyKey, expected)
204+
return sarifutils.GetResultRuleId(actual) == sarifutils.GetResultRuleId(expected) && sarifutils.GetResultProperty(sarifparser.WatchSarifPropertyKey, actual) == sarifutils.GetResultProperty(sarifparser.WatchSarifPropertyKey, expected)
206205
}
207206

208207
func getResultId(result *sarif.Result) string {

0 commit comments

Comments
 (0)