From 56d1128bea6b750a0ea221739bc499ccbee17d17 Mon Sep 17 00:00:00 2001 From: Jason Lynch Date: Tue, 30 May 2023 14:14:19 -0400 Subject: [PATCH] fix: CLOUD-1530 remove support for old controls format This PR removes the code path that supported the old controls format. We only needed this backwards compatibility temporarily when we were migrating to the new controls format. This also fixes a bug where some newly-added rule result fields were not added to the custom unmarshal function and were missing in unmarshalled results. --- .../unreleased/Removed-20230530-141448.yaml | 3 + .../metadata/rules/snyk_007/metadata.json | 10 -- pkg/models/compat.go | 94 ------------------- pkg/models/json_test.go | 82 ---------------- pkg/policy/base.go | 79 ++++------------ test/examples.json | 14 ++- test/fuguerules.json | 6 ++ test/regression.json | 1 + 8 files changed, 39 insertions(+), 250 deletions(-) create mode 100644 changes/unreleased/Removed-20230530-141448.yaml delete mode 100644 examples/metadata/rules/snyk_007/metadata.json delete mode 100644 pkg/models/compat.go delete mode 100644 pkg/models/json_test.go diff --git a/changes/unreleased/Removed-20230530-141448.yaml b/changes/unreleased/Removed-20230530-141448.yaml new file mode 100644 index 00000000..2f1046cd --- /dev/null +++ b/changes/unreleased/Removed-20230530-141448.yaml @@ -0,0 +1,3 @@ +kind: Removed +body: support for old controls format +time: 2023-05-30T14:14:48.842054-04:00 diff --git a/examples/metadata/rules/snyk_007/metadata.json b/examples/metadata/rules/snyk_007/metadata.json deleted file mode 100644 index 3f617052..00000000 --- a/examples/metadata/rules/snyk_007/metadata.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "metadata": { - "description": "This is rule 7, it uses an old control format", - "controls": { - "Simpsons": { - "v1.0.0": ["Homer", "Marge"] - } - } - } -} diff --git a/pkg/models/compat.go b/pkg/models/compat.go deleted file mode 100644 index 48c35024..00000000 --- a/pkg/models/compat.go +++ /dev/null @@ -1,94 +0,0 @@ -package models - -import ( - "encoding/json" - "fmt" - "sort" -) - -// Compatibility type to unmarshal controls in the old (map-based) as well -// as the new (array-based) format. -func ParseControls(data interface{}) ([]string, error) { - controls := []string{} - if familyMap, ok := data.(map[string]interface{}); ok { - families := []string{} - for family := range familyMap { - families = append(families, family) - } - sort.Strings(families) - for _, family := range families { - if versionMap, ok := familyMap[family].(map[string]interface{}); ok { - versions := []string{} - for version := range versionMap { - versions = append(versions, version) - } - sort.Strings(versions) - for _, version := range versions { - if sections, ok := versionMap[version].([]interface{}); ok { - for _, section := range sections { - if section, ok := section.(string); ok { - control := fmt.Sprintf("%s_%s_%s", family, version, section) - controls = append(controls, control) - } else { - return nil, fmt.Errorf("controls section should be string") - } - } - } else { - return nil, fmt.Errorf("controls version should contain array") - } - } - } else { - return nil, fmt.Errorf("controls family should contain object") - } - } - } else if controlSlice, ok := data.([]interface{}); ok { - for _, control := range controlSlice { - if control := control.(string); ok { - controls = append(controls, control) - } else { - return nil, fmt.Errorf("control should be string") - } - } - } else if data != nil { - return nil, fmt.Errorf("controls should contain array or object") - } - return controls, nil -} - -func (r *RuleResults) UnmarshalJSON(data []byte) error { - compat := struct { - Id string `json:"id,omitempty"` - Title string `json:"title,omitempty"` - Platform []string `json:"platform,omitempty"` - Description string `json:"description,omitempty"` - References []RuleResultsReference `json:"references,omitempty"` - Category string `json:"category,omitempty"` - Labels []string `json:"labels,omitempty"` - ServiceGroup string `json:"service_group,omitempty"` - Controls interface{} `json:"controls"` - ResourceTypes []string `json:"resource_types,omitempty"` - Results []RuleResult `json:"results"` - Errors []string `json:"errors,omitempty"` - Package_ string `json:"package,omitempty"` - }{} - err := json.Unmarshal(data, &compat) - if err != nil { - return err - } - r.Id = compat.Id - r.Title = compat.Title - r.Platform = compat.Platform - r.Description = compat.Description - r.References = compat.References - r.Category = compat.Category - r.Labels = compat.Labels - r.ServiceGroup = compat.ServiceGroup - if r.Controls, err = ParseControls(compat.Controls); err != nil { - return err - } - r.ResourceTypes = compat.ResourceTypes - r.Results = compat.Results - r.Errors = compat.Errors - r.Package_ = compat.Package_ - return nil -} diff --git a/pkg/models/json_test.go b/pkg/models/json_test.go deleted file mode 100644 index 2011cbb7..00000000 --- a/pkg/models/json_test.go +++ /dev/null @@ -1,82 +0,0 @@ -package models - -import ( - "encoding/json" - "fmt" - "testing" - - "github.com/stretchr/testify/assert" -) - -type mockReport struct { - RuleResults map[string]RuleResults `json:"rule_results"` -} - -func TestUnmarshalRuleResults(t *testing.T) { - tests := []struct { - serialized string - expected mockReport - }{ - { - `{ - "rule_results": { - "foo": { - "controls": { - "CIS-AWS": { - "v1.3.0": [ - "5.1", - "5.2" - ], - "v1.4.0": [ - "6.7" - ] - } - } - } - } - }`, - mockReport{ - RuleResults: map[string]RuleResults{ - "foo": { - Controls: []string{ - "CIS-AWS_v1.3.0_5.1", - "CIS-AWS_v1.3.0_5.2", - "CIS-AWS_v1.4.0_6.7", - }, - }, - }, - }, - }, - { - `{ - "rule_results": { - "foo": { - "controls": [ - "CIS-AWS_v1.3.0_5.1", - "CIS-AWS_v1.3.0_5.2", - "CIS-AWS_v1.4.0_6.7" - ] - } - } - }`, - mockReport{ - RuleResults: map[string]RuleResults{ - "foo": { - Controls: []string{ - "CIS-AWS_v1.3.0_5.1", - "CIS-AWS_v1.3.0_5.2", - "CIS-AWS_v1.4.0_6.7", - }, - }, - }, - }, - }, - } - for i, test := range tests { - t.Run(fmt.Sprintf("case %d", i), func(t *testing.T) { - actual := mockReport{} - assert.NoError(t, json.Unmarshal([]byte(test.serialized), &actual)) - assert.Equal(t, test.expected, actual) - }) - } -} diff --git a/pkg/policy/base.go b/pkg/policy/base.go index 0f64b0ad..ae3f631e 100644 --- a/pkg/policy/base.go +++ b/pkg/policy/base.go @@ -43,6 +43,7 @@ const resourcesRuleName = "resources" const resourceTypeRuleName = "resource_type" const inputTypeRuleName = "input_type" const multipleResourceType = "MULTIPLE" +const defaultKind = "vulnerability" // SupportedInputTypes contains all of the input types that this package officially // supports. @@ -139,53 +140,19 @@ var remediationKeys = map[string]string{ } type Metadata struct { - ID string `json:"id"` - Title string `json:"title"` - Description string `json:"description"` - Platform []string `json:"platform"` - Remediation map[string]string `json:"remediation"` - References map[string][]MetadataReference `json:"references"` - Category string `json:"category"` - Labels []string `json:"labels,omitempty"` - ServiceGroup string `json:"service_group"` - Controls []string `json:"controls"` - Severity string `json:"severity"` - Product []string `json:"product"` - Kind string `json:"kind"` -} - -// Auxiliary parsing type. -type metadataCompat struct { - ID string `rego:"id"` - Title string `rego:"title"` - Description string `rego:"description"` - Platform []string `rego:"platform"` - Remediation map[string]string `rego:"remediation"` - References map[string][]MetadataReference `rego:"references"` - Category string `rego:"category"` - Labels []string `rego:"labels"` - ServiceGroup string `rego:"service_group"` - Controls interface{} `rego:"controls"` - Severity string `rego:"severity"` - Product []string `rego:"product"` - Kind string `rego:"kind"` -} - -func (compat metadataCompat) ToMetadata() (meta Metadata, err error) { - meta.ID = compat.ID - meta.Title = compat.Title - meta.Description = compat.Description - meta.Platform = compat.Platform - meta.Remediation = compat.Remediation - meta.References = compat.References - meta.Category = compat.Category - meta.Labels = compat.Labels - meta.ServiceGroup = compat.ServiceGroup - meta.Controls, err = models.ParseControls(compat.Controls) - meta.Severity = compat.Severity - meta.Product = compat.Product - meta.Kind = compat.Kind - return + ID string `json:"id" rego:"id"` + Title string `json:"title" rego:"title"` + Description string `json:"description" rego:"description"` + Platform []string `json:"platform" rego:"platform"` + Remediation map[string]string `json:"remediation" rego:"remediation"` + References map[string][]MetadataReference `json:"references" rego:"references"` + Category string `json:"category" rego:"category"` + Labels []string `json:"labels,omitempty" rego:"labels"` + ServiceGroup string `json:"service_group" rego:"service_group"` + Controls []string `json:"controls" rego:"controls"` + Severity string `json:"severity" rego:"severity"` + Product []string `json:"product" rego:"product"` + Kind string `json:"kind" rego:"kind"` } func (m Metadata) RemediationFor(inputType string) string { @@ -348,26 +315,17 @@ func (p *BasePolicy) Metadata( return *p.cachedMetadata, nil } m := Metadata{} - if p.metadataRule.name == "" { - p.cachedMetadata = &m - return m, nil - } switch p.metadataRule.name { case "metadata": if err := state.Query( ctx, rego.Query{Query: p.metadataRule.query()}, func(val ast.Value) error { - compat := metadataCompat{} - err := rego.Bind(val, &compat) + err := rego.Bind(val, &m) if err != nil { return err } - if compat.Kind == "" { - compat.Kind = "vulnerability" - } - m, err = compat.ToMetadata() - return err + return nil }, ); err != nil { return m, err @@ -397,10 +355,13 @@ func (p *BasePolicy) Metadata( ); err != nil { return m, err } - + case "": // noop when no metadata rule is defined default: return m, fmt.Errorf("Unrecognized metadata rule: %s", p.metadataRule.name) } + if m.Kind == "" { + m.Kind = defaultKind + } p.cachedMetadata = &m return m, nil } diff --git a/test/examples.json b/test/examples.json index 8b9a7991..494cfda8 100644 --- a/test/examples.json +++ b/test/examples.json @@ -684,6 +684,7 @@ "package": "data.rules.snyk_001.tf" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -829,6 +830,7 @@ "package": "data.rules.snyk_002.tf" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -908,6 +910,7 @@ "package": "data.rules.snyk_003.tf" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -1073,6 +1076,7 @@ "package": "data.rules.snyk_004.tf" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -1312,6 +1316,7 @@ "package": "data.rules.snyk_005.tf" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -1555,6 +1560,7 @@ "package": "data.rules.snyk_005b.tf" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -1798,6 +1804,7 @@ "package": "data.rules.snyk_006.tf" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -1869,11 +1876,6 @@ "rule_bundle": { "source": "data" }, - "description": "This is rule 7, it uses an old control format", - "controls": [ - "Simpsons_v1.0.0_Homer", - "Simpsons_v1.0.0_Marge" - ], "resource_types": [ "aws_cloudtrail" ], @@ -1915,6 +1917,7 @@ "package": "data.rules.snyk_008.tf" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -2051,6 +2054,7 @@ "package": "data.rules.snyk_009.tf" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, diff --git a/test/fuguerules.json b/test/fuguerules.json index ac85b527..f5669295 100644 --- a/test/fuguerules.json +++ b/test/fuguerules.json @@ -1,6 +1,7 @@ [ { "id": "FG_R00099", + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -47,6 +48,7 @@ "package": "data.rules.fugue_advanced" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -86,6 +88,7 @@ "package": "data.rules.fugue_simple_allow_boolean" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -127,6 +130,7 @@ "package": "data.rules.fugue_simple_allow_string" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -166,6 +170,7 @@ "package": "data.rules.fugue_simple_deny_boolean" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, @@ -311,6 +316,7 @@ "package": "data.rules.fugue_simple_deny_info" }, { + "kind": "vulnerability", "rule_bundle": { "source": "data" }, diff --git a/test/regression.json b/test/regression.json index 44ce2676..97fcfe1d 100644 --- a/test/regression.json +++ b/test/regression.json @@ -1,5 +1,6 @@ [ { + "kind": "vulnerability", "rule_bundle": { "source": "data" },