Skip to content

Commit 924d5ff

Browse files
Dboy0ZDevDboy0ZDev
committed
Added removal of debug prints, subcomponent checks, eventType accepted handling, time nil pointer check, http request improvements and url checks
1 parent d221109 commit 924d5ff

5 files changed

Lines changed: 84 additions & 26 deletions

File tree

‎normalize/sbom_graph.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,7 @@ func validateVexReportOpenVEX(report *ov.VEX) error {
157157
if report.Author == "" {
158158
return fmt.Errorf("invalid OpenVEX report: missing author")
159159
}
160-
if report.Timestamp.IsZero() {
160+
if report.Timestamp != nil && report.Timestamp.IsZero() {
161161
return fmt.Errorf("invalid OpenVEX report: missing timestamp")
162162
}
163163
if report.Version == 0 {

‎services/scan_service.go‎

Lines changed: 36 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,10 @@ type scanService struct {
6868
}
6969

7070
var newGitHubClient = func() *github.Client {
71-
return github.NewClient(nil)
71+
return github.NewClient(&http.Client{
72+
Transport: utils.EgressTransport,
73+
Timeout: 10 * time.Minute,
74+
})
7275
}
7376

7477
var downloadRawFileFn = DownloadRawFile
@@ -877,10 +880,6 @@ func (s *scanService) ScanSBOMWithoutSaving(ctx context.Context, bom *cyclonedx.
877880

878881
func (s *scanService) FetchOpenVexFromGitHub(ctx context.Context, targetURL string) (vexReports []*normalize.VexReportOpenVEX, err error) {
879882
client := newGitHubClient()
880-
githubDomain := "https://github.com"
881-
if !strings.HasPrefix(targetURL, githubDomain) {
882-
return nil, fmt.Errorf("invalid github repository url")
883-
}
884883
owner, repo, err := ParseGitHubURL(targetURL)
885884
if err != nil {
886885
return nil, err
@@ -918,6 +917,7 @@ func (s *scanService) FetchOpenVexFromGitHub(ctx context.Context, targetURL stri
918917
}
919918

920919
content, err := downloadRawFileFn(
920+
ctx,
921921
owner,
922922
repo,
923923
branch,
@@ -947,11 +947,23 @@ func ParseGitHubURL(rawURL string) (owner string, repo string, err error) {
947947
if err != nil {
948948
return "", "", err
949949
}
950+
const githubDomain = "github.com"
951+
if u.Host != githubDomain {
952+
return "", "", fmt.Errorf("invalid github repository url")
953+
}
950954
parts := strings.Split(strings.Trim(u.Path, "/"), "/")
951-
return parts[0], parts[1], nil
955+
if len(parts) < 2 {
956+
return "", "", fmt.Errorf("invalid github repository url path: expected /{owner}/{repo}, got %q", u.Path)
957+
}
958+
owner = parts[0]
959+
repo = strings.TrimSuffix(parts[1], ".git")
960+
if owner == "" || repo == "" {
961+
return "", "", fmt.Errorf("invalid github repository url path: expected non-empty owner and repo, got %q", u.Path)
962+
}
963+
return owner, repo, nil
952964
}
953965

954-
func DownloadRawFile(owner, repo, branch, filePath string) ([]byte, error) {
966+
func DownloadRawFile(ctx context.Context, owner, repo, branch, filePath string) ([]byte, error) {
955967

956968
rawURL := fmt.Sprintf(
957969
"https://raw.githubusercontent.com/%s/%s/%s/%s",
@@ -960,11 +972,25 @@ func DownloadRawFile(owner, repo, branch, filePath string) ([]byte, error) {
960972
branch,
961973
filePath,
962974
)
963-
resp, err := http.Get(rawURL)
975+
resp, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil)
964976
if err != nil {
965977
return nil, err
966978
}
967979
defer resp.Body.Close()
968-
return io.ReadAll(resp.Body)
969-
980+
switch resp.Response.StatusCode {
981+
case http.StatusOK:
982+
file, err := io.ReadAll(resp.Body)
983+
if err != nil {
984+
return nil, fmt.Errorf("401 Unauthorized")
985+
}
986+
return file, nil
987+
case http.StatusNotFound:
988+
return nil, fmt.Errorf("404 Source not found")
989+
case http.StatusUnauthorized:
990+
return nil, fmt.Errorf("401 Unauthorized")
991+
case http.StatusInternalServerError:
992+
return nil, fmt.Errorf("500 Internal Server error")
993+
default:
994+
return nil, fmt.Errorf("Unexpected status: %d\n", resp.Response.StatusCode)
995+
}
970996
}

‎services/scan_service_test.go‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ func TestFetchOpenVexFromGitHub(t *testing.T) {
332332
}
333333

334334
calls := 0
335-
downloadRawFileFn = func(owner, repo, branch, filePath string) ([]byte, error) {
335+
downloadRawFileFn = func(ctx context.Context, owner, repo, branch, filePath string) ([]byte, error) {
336336
calls++
337337
assert.Equal(t, "octo-org", owner)
338338
assert.Equal(t, "openvex-repo", repo)
@@ -390,7 +390,7 @@ func TestFetchOpenVexFromGitHub(t *testing.T) {
390390
}
391391

392392
calls := 0
393-
downloadRawFileFn = func(owner, repo, branch, filePath string) ([]byte, error) {
393+
downloadRawFileFn = func(ctx context.Context, owner, repo, branch, filePath string) ([]byte, error) {
394394
calls++
395395
assert.Equal(t, "octo-org", owner)
396396
assert.Equal(t, "multi-vex-repo", repo)

‎services/vex_rule_service.go‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -545,7 +545,6 @@ func (s *VEXRuleService) parseVEXRulesFromOpenVEXReport(ctx context.Context, ass
545545
var err error
546546
var componentPurl packageurl.PackageURL
547547
if product.Identifiers != nil && product.Identifiers[ov.PURL] != "" {
548-
fmt.Println(product.Identifiers[ov.PURL])
549548
componentPurl, err = packageurl.FromString(product.Identifiers[ov.PURL])
550549
} else if product.ID != "" {
551550
componentPurl, err = packageurl.FromString(product.ID)
@@ -559,7 +558,14 @@ func (s *VEXRuleService) parseVEXRulesFromOpenVEXReport(ctx context.Context, ass
559558
continue
560559
}
561560

562-
justification := statement.ImpactStatement
561+
var justification string
562+
switch statement.Status {
563+
case ov.StatusAffected:
564+
justification = statement.ActionStatement
565+
case ov.StatusNotAffected:
566+
justification = statement.ImpactStatement
567+
}
568+
563569
mechanicalJustification := dtos.MechanicalJustificationType(statement.Justification)
564570

565571
eventType, err := mapOpenVEXToEventType(&statement)
@@ -574,7 +580,7 @@ func (s *VEXRuleService) parseVEXRulesFromOpenVEXReport(ctx context.Context, ass
574580
componentPurlStr = componentPurl.String()
575581
}
576582

577-
if product.Subcomponents != nil {
583+
if len(product.Subcomponents) > 0 {
578584
for _, subcomponent := range product.Subcomponents {
579585
var subcomponentPurl packageurl.PackageURL
580586
subcomponentPurl, err = packageurl.FromString(subcomponent.ID)
@@ -632,9 +638,12 @@ func mapOpenVEXToEventType(s *ov.Statement) (dtos.VulnEventType, error) {
632638
case ov.StatusNotAffected:
633639
return dtos.EventTypeFalsePositive, nil
634640
case ov.StatusAffected:
635-
return dtos.EventTypeAccepted, nil
641+
if s.ActionStatement != "" {
642+
return dtos.EventTypeComment, nil
643+
}
644+
return "", fmt.Errorf("vulnerability analysis state is exploitable, no event type mapping")
636645
default:
637-
return "", fmt.Errorf("unknown vulnerability analysis state: %s", s.Status)
646+
return "", fmt.Errorf("unsupported OpenVEX vulnerability analysis state: %s", s.Status)
638647
}
639648
}
640649

‎services/vex_rule_service_test.go‎

Lines changed: 31 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -847,7 +847,7 @@ func TestParseVEXRulesInBOM_ComponentPurlWithEncodedAtSign(t *testing.T) {
847847
vexRuleRepo.AssertExpectations(t)
848848
}
849849

850-
func TestParseVEXRulesFromOpenVEXReport(t *testing.T) {
850+
func TestParseVEXRulesFromOpenVEXReport_SelectValidProductID(t *testing.T) {
851851
assetID := uuid.New()
852852
service := NewVEXRuleService(nil, nil, nil)
853853

@@ -947,7 +947,7 @@ func TestParseVEXRulesFromOpenVEXReport_NormalAndMultipleStatements(t *testing.T
947947
Name: "CVE-2024-1111",
948948
},
949949
Status: ov.StatusNotAffected,
950-
Justification: "component_not_present",
950+
Justification: ov.ComponentNotPresent,
951951
Products: []ov.Product{
952952
{
953953
Component: ov.Component{
@@ -965,7 +965,7 @@ func TestParseVEXRulesFromOpenVEXReport_NormalAndMultipleStatements(t *testing.T
965965
Name: "CVE-2024-2222",
966966
},
967967
Status: ov.StatusNotAffected,
968-
Justification: "component_not_present",
968+
Justification: ov.ComponentNotPresent,
969969
Products: []ov.Product{
970970
{
971971
Component: ov.Component{
@@ -990,6 +990,24 @@ func TestParseVEXRulesFromOpenVEXReport_NormalAndMultipleStatements(t *testing.T
990990
},
991991
},
992992
},
993+
{
994+
ID: "stmt-3",
995+
Vulnerability: ov.Vulnerability{
996+
Name: "CVE-2024-3333",
997+
},
998+
Status: ov.StatusAffected,
999+
ActionStatement: "Update",
1000+
Products: []ov.Product{
1001+
{
1002+
Component: ov.Component{
1003+
ID: "pkg:golang/app@1.0",
1004+
Identifiers: map[ov.IdentifierType]string{
1005+
ov.PURL: "pkg:golang/app@1.0",
1006+
},
1007+
},
1008+
},
1009+
},
1010+
},
9931011
},
9941012
},
9951013
}
@@ -998,19 +1016,24 @@ func TestParseVEXRulesFromOpenVEXReport_NormalAndMultipleStatements(t *testing.T
9981016
assert.NoError(t, err)
9991017

10001018
expected := []struct {
1001-
cve string
1002-
path []string
1019+
cve string
1020+
path []string
1021+
mechanicalJustification string
1022+
eventType dtos.VulnEventType
10031023
}{
1004-
{cve: "CVE-2024-1111", path: []string{"pkg:golang/app@1.0"}},
1005-
{cve: "CVE-2024-2222", path: []string{"pkg:golang/lib@2.0", dtos.PathPatternWildcard, "pkg:golang/lib/sub@2.0"}},
1006-
{cve: "CVE-2024-2222", path: []string{"pkg:golang/app@1.0"}},
1024+
{cve: "CVE-2024-1111", path: []string{"pkg:golang/app@1.0"}, mechanicalJustification: string(ov.ComponentNotPresent), eventType: dtos.EventTypeFalsePositive},
1025+
{cve: "CVE-2024-2222", path: []string{"pkg:golang/lib@2.0", dtos.PathPatternWildcard, "pkg:golang/lib/sub@2.0"}, mechanicalJustification: string(ov.ComponentNotPresent), eventType: dtos.EventTypeFalsePositive},
1026+
{cve: "CVE-2024-2222", path: []string{"pkg:golang/app@1.0"}, mechanicalJustification: string(ov.ComponentNotPresent), eventType: dtos.EventTypeFalsePositive},
1027+
{cve: "CVE-2024-3333", path: []string{"pkg:golang/app@1.0"}, mechanicalJustification: "", eventType: dtos.EventTypeComment},
10071028
}
10081029

10091030
assert.Len(t, rules, len(expected), "number of generated rules should match expected")
10101031

10111032
// We check by order, results and expected results have to line up for this test
10121033
for i, exp := range expected {
10131034
assert.Equal(t, exp.path, []string(rules[i].PathPattern), "path pattern for %s", exp.cve)
1035+
assert.Equal(t, exp.mechanicalJustification, string(rules[i].MechanicalJustification), "justification for %s", exp.cve)
1036+
assert.Equal(t, exp.eventType, rules[i].EventType, "eventType for %s", exp.cve)
10141037
}
10151038
}
10161039

0 commit comments

Comments
 (0)