Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(issueVariant): allow for creation of issue variant with only a severity rating #362

Merged
merged 7 commits into from
Nov 19, 2024
19 changes: 17 additions & 2 deletions internal/api/graphql/graph/model/models.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ func NewPage(p *entity.Page) *Page {
func NewSeverity(sev entity.Severity) *Severity {
severity, _ := SeverityValue(sev.Value)

if severity == "unknown" {
if severity == "unknown" || sev.Cvss == (entity.Cvss{}) {
return &Severity{
Value: &severity,
Score: &sev.Score,
Expand Down Expand Up @@ -145,9 +145,24 @@ func NewSeverity(sev entity.Severity) *Severity {
}

func NewSeverityEntity(severity *SeverityInput) entity.Severity {
if severity == nil || severity.Vector == nil {
if severity == nil || (severity.Rating == nil && severity.Vector == nil) {
// no severity information was passed
return entity.Severity{}
}
if severity.Rating != nil && severity.Vector != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a vector is there, let's take the vector by default and simply overrule the "normal" severity.

@MR2011 @dorneanu WDYT?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated it now so that it just uses the vector and ignores the rating when both are passed.

// both were passed - check if they match
vectorSeverity := entity.NewSeverity(*severity.Vector)
ratingSeverity := entity.NewSeverityFromRating(entity.SeverityValues(*severity.Rating))
if vectorSeverity.Value != ratingSeverity.Value {
return entity.Severity{}
}
return vectorSeverity
}
if (severity.Vector == nil || *severity.Vector == "") && severity.Rating != nil {
// only rating was passed
return entity.NewSeverityFromRating(entity.SeverityValues(*severity.Rating))
}
// only vector was passed
return entity.NewSeverity(*severity.Vector)
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# SPDX-FileCopyrightText: 2024 SAP SE or an SAP affiliate company and Greenhouse contributors
# SPDX-License-Identifier: Apache-2.0

mutation ($input: IssueVariantInput!) {
createIssueVariant (
input: $input
) {
id
secondaryName
description
severity {
value
score
}
issueRepositoryId
issueId
}
}
1 change: 1 addition & 0 deletions internal/api/graphql/graph/schema/common.graphqls
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,7 @@ type Severity {

input SeverityInput {
vector: String
rating: SeverityValues
}

type FilterItem {
Expand Down
29 changes: 26 additions & 3 deletions internal/database/mariadb/entity.go
Original file line number Diff line number Diff line change
Expand Up @@ -317,14 +317,21 @@ type IssueVariantRow struct {
}

func (ivr *IssueVariantRow) AsIssueVariant(repository *entity.IssueRepository) entity.IssueVariant {
var severity entity.Severity
if ivr.Vector.String == "" {
severity = entity.NewSeverityFromRating(entity.SeverityValues(ivr.Rating.String))
} else {
severity = entity.NewSeverity(GetStringValue(ivr.Vector))
}

return entity.IssueVariant{
Id: GetInt64Value(ivr.Id),
IssueRepositoryId: GetInt64Value(ivr.IssueRepositoryId),
IssueRepository: repository,
SecondaryName: GetStringValue(ivr.SecondaryName),
IssueId: GetInt64Value(ivr.IssueId),
Issue: nil,
Severity: entity.NewSeverity(GetStringValue(ivr.Vector)),
Severity: severity,
Description: GetStringValue(ivr.Description),
CreatedAt: GetTimeValue(ivr.CreatedAt),
DeletedAt: GetTimeValue(ivr.DeletedAt),
Expand Down Expand Up @@ -352,14 +359,22 @@ type IssueVariantWithRepository struct {

func (ivwr *IssueVariantWithRepository) AsIssueVariantEntry() entity.IssueVariant {
rep := ivwr.IssueRepositoryRow.AsIssueRepository()

var severity entity.Severity
if ivwr.Vector.String == "" {
severity = entity.NewSeverityFromRating(entity.SeverityValues(ivwr.Rating.String))
} else {
severity = entity.NewSeverity(GetStringValue(ivwr.Vector))
}

return entity.IssueVariant{
Id: GetInt64Value(ivwr.IssueVariantRow.Id),
IssueRepositoryId: GetInt64Value(ivwr.IssueRepositoryId),
IssueRepository: &rep,
SecondaryName: GetStringValue(ivwr.IssueVariantRow.SecondaryName),
IssueId: GetInt64Value(ivwr.IssueId),
Issue: nil,
Severity: entity.NewSeverity(GetStringValue(ivwr.Vector)),
Severity: severity,
Description: GetStringValue(ivwr.Description),
CreatedAt: GetTimeValue(ivwr.IssueVariantRow.CreatedAt),
DeletedAt: GetTimeValue(ivwr.IssueVariantRow.DeletedAt),
Expand All @@ -374,6 +389,14 @@ type ServiceIssueVariantRow struct {

func (siv *ServiceIssueVariantRow) AsServiceIssueVariantEntry() entity.ServiceIssueVariant {
rep := siv.IssueRepositoryRow.AsIssueRepository()

var severity entity.Severity
if siv.Vector.String == "" {
severity = entity.NewSeverityFromRating(entity.SeverityValues(siv.Rating.String))
} else {
severity = entity.NewSeverity(GetStringValue(siv.Vector))
}

return entity.ServiceIssueVariant{
IssueVariant: entity.IssueVariant{
Id: GetInt64Value(siv.IssueVariantRow.Id),
Expand All @@ -382,7 +405,7 @@ func (siv *ServiceIssueVariantRow) AsServiceIssueVariantEntry() entity.ServiceIs
SecondaryName: GetStringValue(siv.IssueVariantRow.SecondaryName),
IssueId: GetInt64Value(siv.IssueId),
Issue: nil,
Severity: entity.NewSeverity(GetStringValue(siv.Vector)),
Severity: severity,
Description: GetStringValue(siv.Description),
CreatedAt: GetTimeValue(siv.IssueVariantRow.CreatedAt),
DeletedAt: GetTimeValue(siv.IssueVariantRow.DeletedAt),
Expand Down
39 changes: 38 additions & 1 deletion internal/e2e/issue_variant_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ var _ = Describe("Creating IssueVariant via API", Label("e2e", "IssueVariants"),
})

Context("and a mutation query is performed", Label("create.graphql"), func() {
It("creates new issueVariant", func() {
It("creates new issueVariant with Vector", func() {
// create a queryCollection (safe to share across requests)
client := graphql.NewClient(fmt.Sprintf("http://localhost:%s/query", cfg.Port))

Expand Down Expand Up @@ -261,6 +261,43 @@ var _ = Describe("Creating IssueVariant via API", Label("e2e", "IssueVariants"),
Expect(*respData.IssueVariant.IssueID).To(Equal(fmt.Sprintf("%d", issueVariant.IssueId)))
Expect(*respData.IssueVariant.Severity.Cvss.Vector).To(Equal(issueVariant.Severity.Cvss.Vector))
})
It("creates new issueVariant with Rating", func() {
// create a queryCollection (safe to share across requests)
client := graphql.NewClient(fmt.Sprintf("http://localhost:%s/query", cfg.Port))

//@todo may need to make this more fault proof?! What if the test is executed from the root dir? does it still work?
b, err := os.ReadFile("../api/graphql/graph/queryCollection/issueVariant/createWithRating.graphql")

Expect(err).To(BeNil())
str := string(b)
req := graphql.NewRequest(str)

req.Var("input", map[string]interface{}{
"secondaryName": issueVariant.SecondaryName,
"description": issueVariant.Description,
"issueRepositoryId": fmt.Sprintf("%d", issueVariant.IssueRepositoryId),
"issueId": fmt.Sprintf("%d", issueVariant.IssueId),
"severity": map[string]string{
"rating": issueVariant.Severity.Value,
},
})

req.Header.Set("Cache-Control", "no-cache")
ctx := context.Background()

var respData struct {
IssueVariant model.IssueVariant `json:"createIssueVariant"`
}
if err := util2.RequestWithBackoff(func() error { return client.Run(ctx, req, &respData) }); err != nil {
logrus.WithError(err).WithField("request", req).Fatalln("Error while unmarshaling")
}

Expect(*respData.IssueVariant.SecondaryName).To(Equal(issueVariant.SecondaryName))
Expect(*respData.IssueVariant.Description).To(Equal(issueVariant.Description))
Expect(*respData.IssueVariant.IssueRepositoryID).To(Equal(fmt.Sprintf("%d", issueVariant.IssueRepositoryId)))
Expect(*respData.IssueVariant.IssueID).To(Equal(fmt.Sprintf("%d", issueVariant.IssueId)))
Expect(string(*respData.IssueVariant.Severity.Value)).To(Equal(issueVariant.Severity.Value))
})
})
})
})
Expand Down
24 changes: 24 additions & 0 deletions internal/entity/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,30 @@ type Cursor struct {
Limit int
}

func NewSeverityFromRating(rating SeverityValues) Severity {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a comment where this values come from. There might also be an adjustment depending on the CVSS version.
https://nvd.nist.gov/vuln-metrics/cvss

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a comment about the CVSS specification with links, but I'm not sure how I would handle doing adjustments based on which CVSS specification since there is no information being passed other than the rating. Should I just do something else here instead?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For now that's okay

// These values are based on the CVSS v3.1 specification
// https://www.first.org/cvss/v3.1/specification-document#Qualitative-Severity-Rating-Scale
// https://nvd.nist.gov/vuln-metrics/cvss
// They are the lower bounds of the CVSS Score ranges that correlate to each given Rating
score := 0.0
switch rating {
case SeverityValuesLow:
score = 0.1
case SeverityValuesMedium:
score = 4.0
case SeverityValuesHigh:
score = 7.0
case SeverityValuesCritical:
score = 9.0
}

return Severity{
Value: string(rating),
Score: score,
Cvss: Cvss{},
}
}

func NewSeverity(url string) Severity {
ev, err := metric.NewEnvironmental().Decode(url)

Expand Down
Loading