From 47d30abe39ee62eb649f1b78297819f0ff0d1de8 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Tue, 5 Jul 2022 16:11:08 +0100 Subject: [PATCH 01/22] (kics auto remediation): first approach --- .../aws/alb_listening_on_http/query.rego | 2 + .../query.rego | 2 +- .../dockerfile/add_instead_of_copy/query.rego | 2 + .../query.rego | 2 - .../alicloud/cmk_is_unusable/query.rego | 4 + .../query.rego | 3 +- .../query.rego | 6 +- .../query.rego | 2 + internal/console/assets/fix-flags.json | 15 ++ internal/console/fix.go | 113 +++++++++ internal/console/flags/fix_flags.go | 7 + internal/console/kics.go | 6 + pkg/engine/inspector.go | 89 ++++--- pkg/engine/inspector_test.go | 7 +- pkg/engine/vulnerability_builder.go | 10 +- pkg/engine/vulnerability_builder_test.go | 18 +- pkg/model/model.go | 2 + pkg/model/summary.go | 4 + pkg/remediation/remediation.go | 138 +++++++++++ pkg/remediation/remediation_utils.go | 121 +++++++++ pkg/remediation/scan.go | 233 ++++++++++++++++++ pkg/scan/scan.go | 1 + pkg/scanner/scanner_test.go | 2 +- test/queries_content_test.go | 1 + test/queries_test.go | 2 +- test/similarity_id_test.go | 2 +- 26 files changed, 730 insertions(+), 64 deletions(-) create mode 100644 internal/console/assets/fix-flags.json create mode 100644 internal/console/fix.go create mode 100644 internal/console/flags/fix_flags.go create mode 100644 pkg/remediation/remediation.go create mode 100644 pkg/remediation/remediation_utils.go create mode 100644 pkg/remediation/scan.go diff --git a/assets/queries/ansible/aws/alb_listening_on_http/query.rego b/assets/queries/ansible/aws/alb_listening_on_http/query.rego index c8b5c92e81b..4027b9d31eb 100644 --- a/assets/queries/ansible/aws/alb_listening_on_http/query.rego +++ b/assets/queries/ansible/aws/alb_listening_on_http/query.rego @@ -19,6 +19,8 @@ CxPolicy[result] { "issueType": "IncorrectValue", "keyExpectedValue": "'aws_elb_application_lb' Protocol should be 'HTTP'", "keyActualValue": "'aws_elb_application_lb' Protocol it's not 'HTTP'", + "remediation": "Protocol: HTTPS", + "remediation_type": "replacement", } } diff --git a/assets/queries/cloudFormation/aws_sam/serverless_api_without_content_encoding/query.rego b/assets/queries/cloudFormation/aws_sam/serverless_api_without_content_encoding/query.rego index 33a6066e1f1..e5657391628 100644 --- a/assets/queries/cloudFormation/aws_sam/serverless_api_without_content_encoding/query.rego +++ b/assets/queries/cloudFormation/aws_sam/serverless_api_without_content_encoding/query.rego @@ -9,7 +9,7 @@ CxPolicy[result] { resource.Type == "AWS::Serverless::Api" properties := resource.Properties - unrecommended_minimum_compression_size(properties.MinimumCompressionSize ) + unrecommended_minimum_compression_size(properties.MinimumCompressionSize) result := { "documentId": input.document[i].id, diff --git a/assets/queries/dockerfile/add_instead_of_copy/query.rego b/assets/queries/dockerfile/add_instead_of_copy/query.rego index add0d6e9153..9d706f02338 100644 --- a/assets/queries/dockerfile/add_instead_of_copy/query.rego +++ b/assets/queries/dockerfile/add_instead_of_copy/query.rego @@ -14,5 +14,7 @@ CxPolicy[result] { "issueType": "IncorrectValue", "keyExpectedValue": sprintf("'COPY' %s", [resource.Value[0]]), "keyActualValue": sprintf("'ADD' %s", [resource.Value[0]]), + "remediation": sprintf("COPY %s", [concat(" ", resource.Value)]), + "remediation_type": "replacement", } } diff --git a/assets/queries/terraform/alicloud/api_gateway_api_protocol_not_https/query.rego b/assets/queries/terraform/alicloud/api_gateway_api_protocol_not_https/query.rego index c11ac249255..cf3a70dcc44 100644 --- a/assets/queries/terraform/alicloud/api_gateway_api_protocol_not_https/query.rego +++ b/assets/queries/terraform/alicloud/api_gateway_api_protocol_not_https/query.rego @@ -17,7 +17,6 @@ CxPolicy[result] { "keyExpectedValue": "'protocol' value should be 'HTTPS'", "keyActualValue": "'protocol' value is 'HTTP' or 'HTTP,HTTPS'", "searchLine": common_lib.build_search_line(["resource", "alicloud_api_gateway_api", name, "request_config","protocol"], []), - } } @@ -35,6 +34,5 @@ CxPolicy[result] { "keyExpectedValue": "'protocol' value should be 'HTTPS'", "keyActualValue": "'protocol' value is 'HTTP' or 'HTTP,HTTPS'", "searchLine": common_lib.build_search_line(["resource", "alicloud_api_gateway_api", name, "request_config", index, "protocol" ], []), - } } diff --git a/assets/queries/terraform/alicloud/cmk_is_unusable/query.rego b/assets/queries/terraform/alicloud/cmk_is_unusable/query.rego index 4828974c244..fee1ee1c6dd 100644 --- a/assets/queries/terraform/alicloud/cmk_is_unusable/query.rego +++ b/assets/queries/terraform/alicloud/cmk_is_unusable/query.rego @@ -17,6 +17,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_kms_key[%s].is_enabled to be set to true", [name]), "keyActualValue": sprintf("alicloud_kms_key[%s].is_enabled is set to false", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_kms_key", name, "is_enabled"], []), + "remediation": "is_enabled = true", + "remediation_type": "replacement", } } @@ -34,5 +36,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_kms_key[%s].is_enabled to be set to true", [name]), "keyActualValue": sprintf("alicloud_kms_key[%s].is_enabled is not set", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_kms_key", name], []), + "remediation": "is_enabled = true", + "remediation_type": "addition", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego index 62fb8639d5a..552c5707fd7 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego @@ -17,7 +17,8 @@ CxPolicy[result] { "keyExpectedValue": "'max_password_age' should be higher than 0 and lower than 91", "keyActualValue": "'max_password_age' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name], []), - + "remediation": "max_password_age = 12", + "remediation_type": "addition", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego index c8013474423..bdecdfc9b63 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego @@ -17,6 +17,8 @@ CxPolicy[result] { "keyExpectedValue": "'minimum_password_length' is defined and set to 14 or above", "keyActualValue": "'minimum_password_length' is lower than 14", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "minimum_password_length"], []), + "remediation": "minimum_password_length = 14", + "remediation_type": "replacement", } } @@ -35,6 +37,8 @@ CxPolicy[result] { "issueType": "MissingAttribute", "keyExpectedValue": "'minimum_password_length' is defined and set to 14 or above ", "keyActualValue": "'minimum_password_length' is not difined", - "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name], []), + "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name], []), + "remediation": "minimum_password_length = 14", + "remediation_type": "addition", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_symbols/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_symbols/query.rego index 2a1bc946fc6..915a0a4d86a 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_symbols/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_symbols/query.rego @@ -17,5 +17,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("resource.alicloud_ram_account_password_policy[%s].require_symbols is set to 'true'", [name]), "keyActualValue": sprintf("resource.alicloud_ram_account_password_policy[%s].require_symbols is configured as 'false'", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "require_symbols"], []), + "remediation": "require_symbols = true", + "remediation_type": "replacement", } } diff --git a/internal/console/assets/fix-flags.json b/internal/console/assets/fix-flags.json new file mode 100644 index 00000000000..95f128c2eae --- /dev/null +++ b/internal/console/assets/fix-flags.json @@ -0,0 +1,15 @@ +{ + "include-ids": { + "flagType": "multiStr", + "shorthandFlag": "", + "defaultValue": "all", + "usage": "which remedations should be remediated \nexample \"f6b7acac2d541d8c15c88d2be51b0e6abd576750b71c580f2e3a9346f7ed0e67,6af5fc5d7c0ad0077348a090f7c09949369d24d5608bbdbd14376a15de62afd1\"" + }, + "results": { + "flagType": "str", + "shorthandFlag": "", + "defaultValue": "", + "usage": "points to the results with remediations" + } + } + \ No newline at end of file diff --git a/internal/console/fix.go b/internal/console/fix.go new file mode 100644 index 00000000000..c29efbee057 --- /dev/null +++ b/internal/console/fix.go @@ -0,0 +1,113 @@ +package console + +import ( + _ "embed" // Embed fix flags + "encoding/json" + "fmt" + "os" + + "github.com/Checkmarx/kics/internal/console/flags" + sentryReport "github.com/Checkmarx/kics/internal/sentry" + "github.com/Checkmarx/kics/pkg/engine/source" + internalPrinter "github.com/Checkmarx/kics/pkg/printer" + "github.com/Checkmarx/kics/pkg/remediation" + "github.com/pkg/errors" + "github.com/rs/zerolog/log" + "github.com/spf13/cobra" +) + +var ( + //go:embed assets/fix-flags.json + fixFlagsListContent string +) + +// NewFixCmd creates a new instance of the fix Command +func NewFixCmd() *cobra.Command { + return &cobra.Command{ + Use: "fix", + Short: "Auto remediates the project", + PersistentPreRunE: func(cmd *cobra.Command, args []string) error { + return preFix(cmd) + }, + RunE: func(cmd *cobra.Command, args []string) error { + return fix(cmd) + }, + } +} + +func initFixCmd(fixCmd *cobra.Command) error { + if err := flags.InitJSONFlags( + fixCmd, + fixFlagsListContent, + false, + source.ListSupportedPlatforms(), + source.ListSupportedCloudProviders()); err != nil { + return err + } + + if err := fixCmd.MarkFlagRequired(flags.Results); err != nil { + sentryReport.ReportSentry(&sentryReport.Report{ + Message: "Failed to add command required flags", + Err: err, + Location: "func initScanCmd()", + }, true) + log.Err(err).Msg("Failed to add command required flags") + } + return nil +} + +func preFix(cmd *cobra.Command) error { + err := flags.Validate() + if err != nil { + return err + } + + err = flags.ValidateQuerySelectionFlags() + if err != nil { + return err + } + err = internalPrinter.SetupPrinter(cmd.InheritedFlags()) + if err != nil { + return errors.New(initError + err.Error()) + } + return err +} + +func fix(cmd *cobra.Command) error { + resultsPath := flags.GetStrFlag(flags.Results) + include := flags.GetMultiStrFlag(flags.IncludeIds) + + content, err := os.ReadFile(resultsPath) + if err != nil { + log.Error().Msgf("failed to read file: %s", err) + return err + } + + results := remediation.Result{} + + err = json.Unmarshal(content, &results) + if err != nil { + log.Error().Msgf("failed to unmarshal file: %s", err) + return err + } + + summary := &remediation.Summary{ + SelectedRemediationsNumber: 0, + ActualRemediationsDoneNumber: 0, + } + + fixs := summary.GetFixs(results, include) + + for filePath := range fixs { + fix := fixs[filePath].(remediation.Fix) + err = summary.RemediateFile(filePath, fix) + if err != nil { + return err + } + } + + fmt.Printf("\nSelected remediations: %d\n", summary.SelectedRemediationsNumber) + fmt.Printf("Remediations done: %d\n", summary.ActualRemediationsDoneNumber) + + return nil +} diff --git a/internal/console/flags/fix_flags.go b/internal/console/flags/fix_flags.go new file mode 100644 index 00000000000..b90c4324929 --- /dev/null +++ b/internal/console/flags/fix_flags.go @@ -0,0 +1,7 @@ +package flags + +// Flags constants for fix +const ( + Results = "results" + IncludeIds = "include-ids" +) diff --git a/internal/console/kics.go b/internal/console/kics.go index a3cad2adb70..bf425bc4176 100644 --- a/internal/console/kics.go +++ b/internal/console/kics.go @@ -42,10 +42,12 @@ func NewKICSCmd() *cobra.Command { func initialize(rootCmd *cobra.Command) error { scanCmd := NewScanCmd() + fixCmd := NewFixCmd() rootCmd.AddCommand(NewVersionCmd()) rootCmd.AddCommand(NewGenerateIDCmd()) rootCmd.AddCommand(scanCmd) rootCmd.AddCommand(NewListPlatformsCmd()) + rootCmd.AddCommand(fixCmd) rootCmd.CompletionOptions.DisableDefaultCmd = true if err := flags.InitJSONFlags( @@ -61,6 +63,10 @@ func initialize(rootCmd *cobra.Command) error { return err } + if err := initFixCmd(fixCmd); err != nil { + return err + } + return initScanCmd(scanCmd) } diff --git a/pkg/engine/inspector.go b/pkg/engine/inspector.go index 719de064937..f18e1ccbfd5 100644 --- a/pkg/engine/inspector.go +++ b/pkg/engine/inspector.go @@ -55,9 +55,9 @@ type QueryLoader struct { type VulnerabilityBuilder func(ctx *QueryContext, tracker Tracker, v interface{}, detector *detector.DetectLine) (model.Vulnerability, error) -type preparedQuery struct { - opaQuery rego.PreparedEvalQuery - metadata model.QueryMetadata +type PreparedQuery struct { + OpaQuery rego.PreparedEvalQuery + Metadata model.QueryMetadata } // Inspector represents a list of compiled queries, a builder for vulnerabilities, an information tracker @@ -78,12 +78,12 @@ type Inspector struct { // QueryContext contains the context where the query is executed, which scan it belongs, basic information of query, // the query compiled and its payload type QueryContext struct { - ctx context.Context + Ctx context.Context scanID string - files map[string]model.FileMetadata - query *preparedQuery + Files map[string]model.FileMetadata + Query *PreparedQuery payload model.Documents - baseScanPaths []string + BaseScanPaths []string } var ( @@ -101,7 +101,8 @@ func NewInspector( tracker Tracker, queryParameters *source.QueryInspectorParameters, excludeResults map[string]bool, - queryTimeout int) (*Inspector, error) { + queryTimeout int, + needsLog bool) (*Inspector, error) { log.Debug().Msg("engine.NewInspector()") metrics.Metric.Start("get_queries") @@ -128,8 +129,10 @@ func NewInspector( metrics.Metric.Stop() - log.Info(). - Msgf("Inspector initialized, number of queries=%d", queryLoader.querySum) + if needsLog { + log.Info(). + Msgf("Inspector initialized, number of queries=%d", queryLoader.querySum) + } lineDetector := detector.NewDetectLine(tracker.GetOutputLines()). Add(helm.DetectKindLine{}, model.KindHELM). @@ -137,7 +140,10 @@ func NewInspector( Add(docker.DetectKindLine{}, model.KindBUILDAH) queryExecTimeout := time.Duration(queryTimeout) * time.Second - log.Info().Msgf("Query execution timeout=%v", queryExecTimeout) + + if needsLog { + log.Info().Msgf("Query execution timeout=%v", queryExecTimeout) + } return &Inspector{ QueryLoader: &queryLoader, @@ -194,7 +200,7 @@ func (c *Inspector) Inspect( for i, queryMeta := range queries { currentQuery <- 1 - queryOpa, err := c.QueryLoader.loadQuery(ctx, &queries[i]) + queryOpa, err := c.QueryLoader.LoadQuery(ctx, &queries[i]) if err != nil { continue } @@ -202,30 +208,30 @@ func (c *Inspector) Inspect( log.Debug().Msgf("Starting to run query %s", queryMeta.Query) queryStartTime := time.Now() - query := &preparedQuery{ - opaQuery: *queryOpa, - metadata: queryMeta, + query := &PreparedQuery{ + OpaQuery: *queryOpa, + Metadata: queryMeta, } vuls, err := c.doRun(&QueryContext{ - ctx: ctx, + Ctx: ctx, scanID: scanID, - files: files.ToMap(), - query: query, + Files: files.ToMap(), + Query: query, payload: combinedFiles, - baseScanPaths: baseScanPaths, + BaseScanPaths: baseScanPaths, }) if err != nil { sentryReport.ReportSentry(&sentryReport.Report{ - Message: fmt.Sprintf("Inspector. query executed with error, query=%s", query.metadata.Query), + Message: fmt.Sprintf("Inspector. query executed with error, query=%s", query.Metadata.Query), Err: err, Location: "func Inspect()", - Platform: query.metadata.Platform, - Metadata: query.metadata.Metadata, - Query: query.metadata.Query, + Platform: query.Metadata.Platform, + Metadata: query.Metadata.Metadata, + Query: query.Metadata.Query, }, true) - c.failedQueries[query.metadata.Query] = err + c.failedQueries[query.Metadata.Query] = err continue } @@ -234,7 +240,7 @@ func (c *Inspector) Inspect( vulnerabilities = append(vulnerabilities, vuls...) - c.tracker.TrackQueryExecution(query.metadata.Aggregation) + c.tracker.TrackQueryExecution(query.Metadata.Aggregation) } return vulnerabilities, nil @@ -278,7 +284,7 @@ func (c *Inspector) GetFailedQueries() map[string]error { } func (c *Inspector) doRun(ctx *QueryContext) ([]model.Vulnerability, error) { - timeoutCtx, cancel := context.WithTimeout(ctx.ctx, c.queryExecTimeout) + timeoutCtx, cancel := context.WithTimeout(ctx.Ctx, c.queryExecTimeout) defer cancel() options := []rego.EvalOption{rego.EvalInput(ctx.payload)} @@ -288,7 +294,7 @@ func (c *Inspector) doRun(ctx *QueryContext) ([]model.Vulnerability, error) { options = append(options, rego.EvalQueryTracer(cov)) } - results, err := ctx.query.opaQuery.Eval(timeoutCtx, options...) + results, err := ctx.Query.OpaQuery.Eval(timeoutCtx, options...) if err != nil { if topdown.IsCancel(err) { return nil, errors.Wrap(err, "query executing timeout exited") @@ -297,24 +303,25 @@ func (c *Inspector) doRun(ctx *QueryContext) ([]model.Vulnerability, error) { return nil, errors.Wrap(err, "failed to evaluate query") } if c.enableCoverageReport && cov != nil { - module, parseErr := ast.ParseModule(ctx.query.metadata.Query, ctx.query.metadata.Content) + module, parseErr := ast.ParseModule(ctx.Query.Metadata.Query, ctx.Query.Metadata.Content) if parseErr != nil { return nil, errors.Wrap(parseErr, "failed to parse coverage module") } c.coverageReport = cov.Report(map[string]*ast.Module{ - ctx.query.metadata.Query: module, + ctx.Query.Metadata.Query: module, }) } log.Trace(). Str("scanID", ctx.scanID). - Msgf("Inspector executed with result %+v, query=%s", results, ctx.query.metadata.Query) + Msgf("Inspector executed with result %+v, query=%s", results, ctx.Query.Metadata.Query) - return c.decodeQueryResults(ctx, results) + return c.DecodeQueryResults(ctx, results) } -func (c *Inspector) decodeQueryResults(ctx *QueryContext, results rego.ResultSet) ([]model.Vulnerability, error) { +// DecodeQueryResults decodes the results into []model.Vulnerability +func (c *Inspector) DecodeQueryResults(ctx *QueryContext, results rego.ResultSet) ([]model.Vulnerability, error) { if len(results) == 0 { return nil, ErrNoResult } @@ -337,21 +344,21 @@ func (c *Inspector) decodeQueryResults(ctx *QueryContext, results rego.ResultSet vulnerability, err := c.vb(ctx, c.tracker, queryResultItem, c.detector) if err != nil { sentryReport.ReportSentry(&sentryReport.Report{ - Message: fmt.Sprintf("Inspector can't save vulnerability, query=%s", ctx.query.metadata.Query), + Message: fmt.Sprintf("Inspector can't save vulnerability, query=%s", ctx.Query.Metadata.Query), Err: err, Location: "func decodeQueryResults()", - Platform: ctx.query.metadata.Platform, - Metadata: ctx.query.metadata.Metadata, - Query: ctx.query.metadata.Query, + Platform: ctx.Query.Metadata.Platform, + Metadata: ctx.Query.Metadata.Metadata, + Query: ctx.Query.Metadata.Query, }, true) - if _, ok := c.failedQueries[ctx.query.metadata.Query]; !ok { - c.failedQueries[ctx.query.metadata.Query] = err + if _, ok := c.failedQueries[ctx.Query.Metadata.Query]; !ok { + c.failedQueries[ctx.Query.Metadata.Query] = err } continue } - file := ctx.files[vulnerability.FileID] + file := ctx.Files[vulnerability.FileID] if ShouldSkipVulnerability(file.Commands, vulnerability.QueryID) { log.Debug().Msgf("Skipping vulnerability in file %s for query '%s':%s", file.FilePath, vulnerability.QueryName, vulnerability.QueryID) continue @@ -445,8 +452,8 @@ func prepareQueries(queries []model.QueryMetadata, commonLibrary source.RegoLibr } } -// Load the query into memory so it can be freed when not used anymore -func (q QueryLoader) loadQuery(ctx context.Context, query *model.QueryMetadata) (*rego.PreparedEvalQuery, error) { +// LoadQuery loads the query into memory so it can be freed when not used anymore +func (q QueryLoader) LoadQuery(ctx context.Context, query *model.QueryMetadata) (*rego.PreparedEvalQuery, error) { opaQuery := rego.PreparedEvalQuery{} platformGeneralQuery, ok := q.platformLibraries[query.Platform] diff --git a/pkg/engine/inspector_test.go b/pkg/engine/inspector_test.go index 5b8b26d3049..f1187ff0166 100644 --- a/pkg/engine/inspector_test.go +++ b/pkg/engine/inspector_test.go @@ -399,6 +399,7 @@ func TestNewInspector(t *testing.T) { // nolint queryFilter source.QueryInspectorParameters excludeResults map[string]bool queryExecTimeout int + needsLog bool } tests := []struct { name string @@ -424,6 +425,7 @@ func TestNewInspector(t *testing.T) { // nolint }, excludeResults: map[string]bool{}, queryExecTimeout: 60, + needsLog: true, }, want: &Inspector{ vb: vbs, @@ -443,7 +445,8 @@ func TestNewInspector(t *testing.T) { // nolint tt.args.tracker, &tt.args.queryFilter, tt.args.excludeResults, - tt.args.queryExecTimeout) + tt.args.queryExecTimeout, + tt.args.needsLog) if (err != nil) != tt.wantErr { t.Errorf("NewInspector() error: got = %v,\n wantErr = %v", err, tt.wantErr) @@ -683,7 +686,7 @@ func newInspectorInstance(t *testing.T, queryPath []string) *Inspector { vb, &tracker.CITracker{}, &source.QueryInspectorParameters{}, - map[string]bool{}, 60, + map[string]bool{}, 60, true, ) require.NoError(t, err) return ins diff --git a/pkg/engine/vulnerability_builder.go b/pkg/engine/vulnerability_builder.go index de453947a3d..5527e5fc12e 100644 --- a/pkg/engine/vulnerability_builder.go +++ b/pkg/engine/vulnerability_builder.go @@ -24,7 +24,7 @@ var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, return model.Vulnerability{}, ErrInvalidResult } - vObj = mergeWithMetadata(vObj, ctx.query.metadata.Metadata) + vObj = mergeWithMetadata(vObj, ctx.Query.Metadata.Metadata) var err error var output []byte @@ -41,7 +41,7 @@ var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, return model.Vulnerability{}, errors.Wrap(err, "failed to recognize file id") } - file, ok := ctx.files[*fileID] + file, ok := ctx.Files[*fileID] if !ok { return model.Vulnerability{}, errors.New("failed to find file from query response") } @@ -49,7 +49,7 @@ var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, logWithFields := log.With(). Str("scanID", ctx.scanID). Str("fileName", file.FilePath). - Str("queryName", ctx.query.metadata.Query). + Str("queryName", ctx.Query.Metadata.Query). Logger() detector.SetupLogs(&logWithFields) @@ -109,7 +109,7 @@ var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, var similarityID *string - similarityID, err = similarity.ComputeSimilarityID(ctx.baseScanPaths, linesVulne.ResolvedFile, queryID, similarityIDLineInfo, searchValue) + similarityID, err = similarity.ComputeSimilarityID(ctx.BaseScanPaths, linesVulne.ResolvedFile, queryID, similarityIDLineInfo, searchValue) if err != nil { logWithFields.Err(err).Send() tracker.FailedComputeSimilarityID() @@ -144,6 +144,8 @@ var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, Value: mustMapKeyToString(vObj, "value"), Output: string(output), CloudProvider: getCloudProvider(platform, overrideKey, vObj, &logWithFields), + Remediation: PtrStringToString(mustMapKeyToString(vObj, "remediation")), + RemediationType: PtrStringToString(mustMapKeyToString(vObj, "remediation_type")), }, nil } diff --git a/pkg/engine/vulnerability_builder_test.go b/pkg/engine/vulnerability_builder_test.go index 22f7e3fcade..7563bd0fb31 100644 --- a/pkg/engine/vulnerability_builder_test.go +++ b/pkg/engine/vulnerability_builder_test.go @@ -29,8 +29,8 @@ var vbTests = []struct { tracker: &tracker.CITracker{}, ctx: &QueryContext{ scanID: "ScanID", - query: &preparedQuery{ - metadata: model.QueryMetadata{ + Query: &PreparedQuery{ + Metadata: model.QueryMetadata{ Metadata: map[string]interface{}{ "key": "123", "severity": model.SeverityInfo, @@ -40,7 +40,7 @@ var vbTests = []struct { Query: "TestQuery", }, }, - files: map[string]model.FileMetadata{ + Files: map[string]model.FileMetadata{ "testV": {}, }, }, @@ -77,8 +77,8 @@ var vbTests = []struct { tracker: &tracker.CITracker{}, ctx: &QueryContext{ scanID: "ScanID", - query: &preparedQuery{ - metadata: model.QueryMetadata{ + Query: &PreparedQuery{ + Metadata: model.QueryMetadata{ Metadata: map[string]interface{}{ "key": "123", "severity": model.SeverityInfo, @@ -94,7 +94,7 @@ var vbTests = []struct { Query: "TestQuery", }, }, - files: map[string]model.FileMetadata{ + Files: map[string]model.FileMetadata{ "testV": {}, }, }, @@ -131,8 +131,8 @@ var vbTests = []struct { tracker: &tracker.CITracker{}, ctx: &QueryContext{ scanID: "ScanID", - query: &preparedQuery{ - metadata: model.QueryMetadata{ + Query: &PreparedQuery{ + Metadata: model.QueryMetadata{ Metadata: map[string]interface{}{ "key": "123", "severity": model.SeverityInfo, @@ -149,7 +149,7 @@ var vbTests = []struct { Query: "TestQuery", }, }, - files: map[string]model.FileMetadata{ + Files: map[string]model.FileMetadata{ "testV": {}, }, }, diff --git a/pkg/model/model.go b/pkg/model/model.go index ee3d45cca7f..36ffe781511 100644 --- a/pkg/model/model.go +++ b/pkg/model/model.go @@ -169,6 +169,8 @@ type Vulnerability struct { Value *string `db:"value" json:"value"` Output string `json:"-"` CloudProvider string `json:"cloud_provider"` + Remediation string `db:"remediation" json:"remediation"` + RemediationType string `db:"remediation_type" json:"remediation_type"` } // QueryConfig is a struct that contains the fileKind and platform of the rego query diff --git a/pkg/model/summary.go b/pkg/model/summary.go index 6d79587ee40..979254b598a 100644 --- a/pkg/model/summary.go +++ b/pkg/model/summary.go @@ -34,6 +34,8 @@ type VulnerableFile struct { KeyExpectedValue string `json:"expected_value"` KeyActualValue string `json:"actual_value"` Value *string `json:"value,omitempty"` + Remediation string `json:"remediation,omitempty"` + RemediationType string `json:"remediation_type,omitempty"` } // QueryResult contains a query that tested positive ID, name, severity and a list of files that tested vulnerable @@ -219,6 +221,8 @@ func CreateSummary(counters Counters, vulnerabilities []Vulnerability, KeyExpectedValue: item.KeyExpectedValue, KeyActualValue: item.KeyActualValue, Value: item.Value, + Remediation: item.Remediation, + RemediationType: item.RemediationType, }) filePaths[resolvedPath] = item.FileName diff --git a/pkg/remediation/remediation.go b/pkg/remediation/remediation.go new file mode 100644 index 00000000000..7fcfa050140 --- /dev/null +++ b/pkg/remediation/remediation.go @@ -0,0 +1,138 @@ +package remediation + +import ( + "os" + "sort" + "strings" + + "github.com/rs/zerolog/log" +) + +type Result struct { + Queries []Query `json:"queries"` +} + +type Query struct { + Files []File `json:"files"` + QueryID string `json:"query_id"` +} + +type File struct { + FilePath string `json:"file_name"` + Line int `json:"line"` + Remediation string `json:"remediation"` + RemediationType string `json:"remediation_type"` + SimilarityID string `json:"similarity_id"` +} + +type Remediation struct { + Line int + Remediation string + SimilarityID string + QueryID string +} + +type Fix struct { + Replacement []Remediation + Addition []Remediation +} + +// RemediateFile remediates the replacements first and secondly, the additions sorted down +func (s *Summary) RemediateFile(filePath string, fix Fix) error { + content, err := os.ReadFile(filePath) + + if err != nil { + log.Error().Msgf("failed to read file: %s", err) + return err + } + + lines := strings.Split(string(content), "\n") + + // do replacements first + if len(fix.Replacement) > 0 { + for i := range fix.Replacement { + r := fix.Replacement[i] + remediatedLines := replacement(r, lines) + if len(remediatedLines) > 0 && willRemediate(remediatedLines, filePath, &r) { + lines = s.writeRemediations(remediatedLines, lines, filePath, r.SimilarityID) + } + } + } + + // do additions after + if len(fix.Addition) > 0 { + // descending order + sort.Slice(fix.Addition, func(i, j int) bool { + return fix.Addition[i].Line > fix.Addition[j].Line + }) + + for i := range fix.Addition { + a := fix.Addition[i] + remediatedLines := addition(a, &lines) + if len(remediatedLines) > 0 && willRemediate(remediatedLines, filePath, &a) { + lines = s.writeRemediations(remediatedLines, lines, filePath, a.SimilarityID) + } + } + } + + return nil +} + +func replacement(r Remediation, lines []string) []string { + originalLine := lines[r.Line-1] + + if strings.Contains(originalLine, r.Remediation) { + log.Info().Msgf("remediation '%s' is already done", r.SimilarityID) + return []string{} + } + + before := getBefore(originalLine) + + // replace the original line with remediation + lines[r.Line-1] = before + r.Remediation + + return lines +} + +func addition(r Remediation, lines *[]string) []string { + fatherNumberLine := r.Line - 1 + + if len(*lines) <= fatherNumberLine+1 { + return []string{} + } + + if strings.Contains((*lines)[fatherNumberLine+1], r.Remediation) { + log.Info().Msgf("remediation '%s' is already done", r.SimilarityID) + return []string{} + } + begin := make([]string, len(*lines)) + end := make([]string, len(*lines)) + + copy(begin, *lines) + copy(end, *lines) + + begin = begin[:fatherNumberLine+1] + end = end[fatherNumberLine+1:] + + before := getBefore((*lines)[fatherNumberLine+1]) + + remediation := begin + remediation = append(remediation, before+r.Remediation) + remediation = append(remediation, end...) + + return remediation +} + +func (s *Summary) writeRemediations(remediatedLines, lines []string, filePath, similarityID string) []string { + remediated := []byte(strings.Join(remediatedLines, "\n")) + + if err := os.WriteFile(filePath, remediated, os.ModePerm); err != nil { + log.Error().Msgf("failed to write file: %s", err) + return lines + } + + log.Info().Msgf("file '%s' was remediated with '%s'", filePath, similarityID) + s.ActualRemediationsDoneNumber++ + + return remediatedLines +} diff --git a/pkg/remediation/remediation_utils.go b/pkg/remediation/remediation_utils.go new file mode 100644 index 00000000000..a8a6a2de00d --- /dev/null +++ b/pkg/remediation/remediation_utils.go @@ -0,0 +1,121 @@ +package remediation + +import ( + "os" + "path/filepath" + "regexp" + "strings" + + "github.com/Checkmarx/kics/pkg/model" + "github.com/Checkmarx/kics/pkg/utils" + "github.com/rs/zerolog/log" +) + +type Summary struct { + SelectedRemediationsNumber int + ActualRemediationsDoneNumber int +} + +// GetFixs collects all the replacements and additions per file +func (s *Summary) GetFixs(results Result, include []string) map[string]interface{} { + fixs := make(map[string]interface{}) + + for i := range results.Queries { + query := results.Queries[i] + + for j := range query.Files { + file := query.Files[j] + + var fix Fix + + if shouldRemediate(&file, include) { + s.SelectedRemediationsNumber++ + + r := &Remediation{ + Line: file.Line, + Remediation: file.Remediation, + SimilarityID: file.SimilarityID, + QueryID: query.QueryID, + } + if file.RemediationType == "replacement" { + fix.Replacement = append(fix.Replacement, *r) + } + + if file.RemediationType == "addition" { + fix.Addition = append(fix.Addition, *r) + } + + if _, ok := fixs[file.FilePath]; !ok { + fixs[file.FilePath] = fix + continue + } + + updatedFix := fixs[file.FilePath].(Fix) + + updatedFix.Addition = append(updatedFix.Addition, fix.Addition...) + updatedFix.Replacement = append(updatedFix.Replacement, fix.Replacement...) + + fixs[file.FilePath] = updatedFix + } + } + } + + return fixs +} + +func shouldRemediate(file *File, include []string) bool { + if len(file.Remediation) > 0 && (include[0] == "all" || utils.Contains(file.SimilarityID, include)) { + return true + } + + return false +} + +func getBefore(line string) string { + re := regexp.MustCompile(`^[\s-]*`) + before := re.FindAll([]byte(line), -1) + + return string(before[0]) +} + +func willRemediate(remediated []string, originalFileName string, remediation *Remediation) bool { + // create temporary file + tmpFile := filepath.Join(os.TempDir(), "temporary-remediation-"+utils.NextRandom()+filepath.Ext(originalFileName)) + f, err := os.OpenFile(tmpFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm) + + if err != nil { + log.Error().Msgf("failed to open temporary file for remediation '%s': %s", remediation.SimilarityID, err) + f.Close() + return false + } + + content := []byte(strings.Join(remediated, "\n")) + + if _, err = f.Write(content); err != nil { + log.Error().Msgf("failed to write temporary file for remediation '%s': %s", remediation.SimilarityID, err) + f.Close() + return false + } + + // scan the temporary file to verify if the remediation removed the result + results, err := scanTmpFile(tmpFile, remediation.QueryID, content) + + if err != nil { + log.Error().Msgf("failed to get results of query %s: %s", remediation.QueryID, err) + return false + } + + return removedSimilarityID(results, remediation.SimilarityID) +} + +func removedSimilarityID(results []model.Vulnerability, similarity string) bool { + for i := range results { + result := results[i] + + if result.SimilarityID == similarity { + log.Info().Msgf("failed to remediate '%s'", similarity) + return false + } + } + return true +} diff --git a/pkg/remediation/scan.go b/pkg/remediation/scan.go new file mode 100644 index 00000000000..b6c345b054e --- /dev/null +++ b/pkg/remediation/scan.go @@ -0,0 +1,233 @@ +package remediation + +import ( + "context" + "encoding/json" + "errors" + "os" + "time" + + "github.com/Checkmarx/kics/pkg/engine" + "github.com/Checkmarx/kics/pkg/kics" + "github.com/Checkmarx/kics/pkg/model" + "github.com/open-policy-agent/opa/topdown" + + "github.com/Checkmarx/kics/internal/console/flags" + "github.com/Checkmarx/kics/internal/tracker" + "github.com/Checkmarx/kics/pkg/engine/source" + "github.com/Checkmarx/kics/pkg/parser" + buildahParser "github.com/Checkmarx/kics/pkg/parser/buildah" + dockerParser "github.com/Checkmarx/kics/pkg/parser/docker" + protoParser "github.com/Checkmarx/kics/pkg/parser/grpc" + jsonParser "github.com/Checkmarx/kics/pkg/parser/json" + terraformParser "github.com/Checkmarx/kics/pkg/parser/terraform" + yamlParser "github.com/Checkmarx/kics/pkg/parser/yaml" + "github.com/Checkmarx/kics/pkg/utils" + "github.com/open-policy-agent/opa/rego" + "github.com/rs/zerolog/log" +) + +type runQueryInfo struct { + payload model.Documents + query *engine.PreparedQuery + inspector *engine.Inspector + tmpFile string + files model.FileMetadatas +} + +func scanTmpFile(tmpFile, queryID string, remediated []byte) ([]model.Vulnerability, error) { + // get payload + files, err := getPayload(tmpFile, remediated) + + os.Remove(tmpFile) + + if err != nil { + log.Err(err) + return []model.Vulnerability{}, err + } + + payload := files.Combine(false) + + // init scan + inspector, err := initScan(queryID) + + if err != nil { + log.Err(err) + return []model.Vulnerability{}, err + } + + // load query + query, err := loadQuery(inspector, queryID) + + if err != nil { + log.Err(err) + return []model.Vulnerability{}, err + } + + // run query + info := &runQueryInfo{ + payload: payload, + query: query, + inspector: inspector, + tmpFile: tmpFile, + files: files, + } + + return runQuery(info), nil +} + +func getPayload(filePath string, content []byte) (model.FileMetadatas, error) { + ext := utils.GetExtension(filePath) + var p []*parser.Parser + var err error + + switch ext { + case ".dockerfile", "Dockerfile", "possibleDockerfile", ".ubi8", ".debian": + p, err = parser.NewBuilder().Add(&dockerParser.Parser{}).Build([]string{""}, []string{""}) + + case ".tf": + p, err = parser.NewBuilder().Add(terraformParser.NewDefault()).Build([]string{""}, []string{""}) + + case ".proto": + p, err = parser.NewBuilder().Add(&protoParser.Parser{}).Build([]string{""}, []string{""}) + + case ".yaml", ".yml": + p, err = parser.NewBuilder().Add(&yamlParser.Parser{}).Build([]string{""}, []string{""}) + + case ".json": + p, err = parser.NewBuilder().Add(&jsonParser.Parser{}).Build([]string{""}, []string{""}) + + case ".sh": + p, err = parser.NewBuilder().Add(&buildahParser.Parser{}).Build([]string{""}, []string{""}) + } + + if err != nil { + log.Error().Msgf("failed to get parser: %s", err) + return model.FileMetadatas{}, err + } + + if len(p) == 0 { + log.Info().Msg("failed to get parser") + return model.FileMetadatas{}, errors.New("failed to get parser") + } + + documents, er := p[0].Parse(filePath, content) + + if er != nil { + log.Error().Msgf("failed to parse file '%s': %s", filePath, er) + return model.FileMetadatas{}, er + } + + var files model.FileMetadatas + + for _, document := range documents.Docs { + _, err = json.Marshal(document) + if err != nil { + continue + } + + file := model.FileMetadata{ + FilePath: filePath, + Document: kics.PrepareScanDocument(document, documents.Kind), + LineInfoDocument: document, + Commands: p[0].CommentsCommands(filePath, content), + OriginalData: string(content), + } + + files = append(files, file) + } + + return files, nil +} + +func runQuery(r *runQueryInfo) []model.Vulnerability { + queryExecTimeout := time.Duration(flags.GetIntFlag(flags.QueryExecTimeoutFlag)) * time.Second + + timeoutCtx, cancel := context.WithTimeout(context.Background(), queryExecTimeout) + defer cancel() + + options := []rego.EvalOption{rego.EvalInput(r.payload)} + + results, err := r.query.OpaQuery.Eval(timeoutCtx, options...) + + if err != nil { + if topdown.IsCancel(err) { + log.Err(err) + } + + log.Err(err) + } + + ctx := context.Background() + + queryCtx := &engine.QueryContext{ + Ctx: ctx, + Query: r.query, + BaseScanPaths: []string{r.tmpFile}, + Files: r.files.ToMap(), + } + + decoded, err := r.inspector.DecodeQueryResults(queryCtx, results) + + if err != nil { + log.Err(err) + } + + return decoded +} + +func initScan(queryID string) (*engine.Inspector, error) { + queriesSource := source.NewFilesystemSource( + flags.GetMultiStrFlag(flags.QueriesPath), + flags.GetMultiStrFlag(flags.TypeFlag), + flags.GetMultiStrFlag(flags.CloudProviderFlag), + flags.GetStrFlag(flags.LibrariesPath)) + + includeQueries := source.IncludeQueries{ + ByIDs: []string{queryID}, + } + + queryFilter := source.QueryInspectorParameters{ + IncludeQueries: includeQueries, + } + + t, err := tracker.NewTracker(flags.GetIntFlag(flags.PreviewLinesFlag)) + if err != nil { + log.Err(err) + return &engine.Inspector{}, err + } + + ctx := context.Background() + + inspector, err := engine.NewInspector(ctx, + queriesSource, + engine.DefaultVulnerabilityBuilder, + t, + &queryFilter, + make(map[string]bool), + flags.GetIntFlag(flags.QueryExecTimeoutFlag), + false, + ) + + return inspector, err +} + +func loadQuery(inspector *engine.Inspector, queryID string) (*engine.PreparedQuery, error) { + if len(inspector.QueryLoader.QueriesMetadata) == 1 { + queryOpa, err := inspector.QueryLoader.LoadQuery(context.Background(), &inspector.QueryLoader.QueriesMetadata[0]) + + if err != nil { + log.Err(err) + return &engine.PreparedQuery{}, err + } + + query := &engine.PreparedQuery{ + OpaQuery: *queryOpa, + Metadata: inspector.QueryLoader.QueriesMetadata[0], + } + + return query, nil + } + + return &engine.PreparedQuery{}, errors.New("unable to load query" + queryID) +} diff --git a/pkg/scan/scan.go b/pkg/scan/scan.go index 9af75b076bb..c03863e229b 100644 --- a/pkg/scan/scan.go +++ b/pkg/scan/scan.go @@ -68,6 +68,7 @@ func (c *Client) initScan(ctx context.Context) (*executeScanParameters, error) { queryFilter, c.ExcludeResultsMap, c.ScanParams.QueryExecTimeout, + true, ) if err != nil { return nil, err diff --git a/pkg/scanner/scanner_test.go b/pkg/scanner/scanner_test.go index 345536b397b..8aa5375d683 100644 --- a/pkg/scanner/scanner_test.go +++ b/pkg/scanner/scanner_test.go @@ -78,7 +78,7 @@ func createServices(types, cloudProviders []string) (serviceSlice, *storage.Memo inspector, err := engine.NewInspector(context.Background(), querySource, engine.DefaultVulnerabilityBuilder, - t, &source.QueryInspectorParameters{}, map[string]bool{}, 60) + t, &source.QueryInspectorParameters{}, map[string]bool{}, 60, true) if err != nil { return nil, nil, err } diff --git a/test/queries_content_test.go b/test/queries_content_test.go index 52abe08002e..e2db9b50dec 100644 --- a/test/queries_content_test.go +++ b/test/queries_content_test.go @@ -246,6 +246,7 @@ func testQueryHasGoodReturnParams(t *testing.T, entry queryEntry) { //nolint }, map[string]bool{}, 60, + true, ) require.Nil(t, err) require.NotNil(t, inspector) diff --git a/test/queries_test.go b/test/queries_test.go index 9a41e6d17ac..90a6a040999 100644 --- a/test/queries_test.go +++ b/test/queries_test.go @@ -167,7 +167,7 @@ func testQuery(tb testing.TB, entry queryEntry, filesPath []string, expectedVuln ExcludeQueries: source.ExcludeQueries{ByIDs: []string{}, ByCategories: []string{}}, InputDataPath: "", }, - map[string]bool{}, 60) + map[string]bool{}, 60, true) require.Nil(tb, err) require.NotNil(tb, inspector) diff --git a/test/similarity_id_test.go b/test/similarity_id_test.go index 054f2ab8ef8..249993c6f24 100644 --- a/test/similarity_id_test.go +++ b/test/similarity_id_test.go @@ -308,7 +308,7 @@ func createInspectorAndGetVulnerabilities(ctx context.Context, t testing.TB, ExcludeQueries: source.ExcludeQueries{ByIDs: []string{}, ByCategories: []string{}}, InputDataPath: "", }, - map[string]bool{}, 60) + map[string]bool{}, 60, true) require.Nil(t, err) require.NotNil(t, inspector) From 773d0b55e32e604d4fa6df3cb984e17bfc063198 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Wed, 6 Jul 2022 14:45:08 +0100 Subject: [PATCH 02/22] adding tests --- internal/console/assets/fix-flags.json | 4 +- pkg/remediation/remediation_test.go | 143 ++++++++++++++++++ .../{remediation_utils.go => utils.go} | 2 + pkg/remediation/utils_test.go | 113 ++++++++++++++ .../kics_auto_remediation/terraform.tf | 20 +++ 5 files changed, 280 insertions(+), 2 deletions(-) create mode 100644 pkg/remediation/remediation_test.go rename pkg/remediation/{remediation_utils.go => utils.go} (98%) create mode 100644 pkg/remediation/utils_test.go create mode 100644 test/fixtures/kics_auto_remediation/terraform.tf diff --git a/internal/console/assets/fix-flags.json b/internal/console/assets/fix-flags.json index 95f128c2eae..29f276bcccc 100644 --- a/internal/console/assets/fix-flags.json +++ b/internal/console/assets/fix-flags.json @@ -3,13 +3,13 @@ "flagType": "multiStr", "shorthandFlag": "", "defaultValue": "all", - "usage": "which remedations should be remediated \nexample \"f6b7acac2d541d8c15c88d2be51b0e6abd576750b71c580f2e3a9346f7ed0e67,6af5fc5d7c0ad0077348a090f7c09949369d24d5608bbdbd14376a15de62afd1\"" + "usage": "which remedations (similarity ids) should be remediated \nexample \"f6b7acac2d541d8c15c88d2be51b0e6abd576750b71c580f2e3a9346f7ed0e67,6af5fc5d7c0ad0077348a090f7c09949369d24d5608bbdbd14376a15de62afd1\"" }, "results": { "flagType": "str", "shorthandFlag": "", "defaultValue": "", - "usage": "points to the results with remediations" + "usage": "points to the JSON results file with remediations" } } \ No newline at end of file diff --git a/pkg/remediation/remediation_test.go b/pkg/remediation/remediation_test.go new file mode 100644 index 00000000000..27f24fe888e --- /dev/null +++ b/pkg/remediation/remediation_test.go @@ -0,0 +1,143 @@ +package remediation + +import ( + "os" + "path/filepath" + "testing" + + "github.com/Checkmarx/kics/internal/console/flags" + "github.com/Checkmarx/kics/pkg/engine/source" + "github.com/Checkmarx/kics/pkg/utils" + "github.com/spf13/cobra" + "github.com/stretchr/testify/require" +) + +func Test_RemediateFile(t *testing.T) { + filePath := "../../test/fixtures/kics_auto_remediation/terraform.tf" + + type args struct { + fix Fix + } + + replacement := &Remediation{ + Line: 5, + Remediation: "require_symbols = true", + SimilarityID: "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", + QueryID: "41a38329-d81b-4be4-aef4-55b2615d3282", + } + + var fix1, fix2, fix3 Fix + fix1.Replacement = append(fix1.Replacement, *replacement) + + addition := &Remediation{ + Line: 1, + Remediation: "minimum_password_length = 14", + SimilarityID: "f282fa13cf5e4ffd4bbb0ee2059f8d0240edcd2ca54b3bb71633145d961de5ce", + QueryID: "a9dfec39-a740-4105-bbd6-721ba163c053", + } + + fix2.Replacement = append(fix2.Replacement, *replacement) + fix2.Addition = append(fix2.Addition, *addition) + + wrongReplacement := &Remediation{ + Line: 5, + Remediation: "require_symbols = false", + SimilarityID: "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", + QueryID: "41a38329-d81b-4be4-aef4-55b2615d3282", + } + + fix3.Replacement = append(fix3.Replacement, *wrongReplacement) + + tests := []struct { + name string + args args + actualRemediationsDoneNumber int + }{ + { + name: "remediate a file with a replacement", + args: args{ + fix: fix1, + }, + actualRemediationsDoneNumber: 1, + }, + { + name: "remediate a file with a replacement and addition", + args: args{ + fix: fix2, + }, + actualRemediationsDoneNumber: 2, + }, + { + name: "remediate a file without any remediation", + args: args{ + fix: Fix{}, + }, + actualRemediationsDoneNumber: 0, + }, + { + name: "remediate a file with a wrong remediation", + args: args{ + fix: fix3, + }, + actualRemediationsDoneNumber: 0, + }, + } + + mockCmd := &cobra.Command{ + Use: "mock", + Short: "Mock cmd", + RunE: func(cmd *cobra.Command, args []string) error { + return nil + }, + } + + data, err := os.ReadFile(filepath.FromSlash("../../internal/console/assets/scan-flags.json")) + require.NoError(t, err) + flags.InitJSONFlags( + mockCmd, + string(data), + true, + source.ListSupportedPlatforms(), + source.ListSupportedCloudProviders(), + ) + + flags.SetMultiStrFlag(flags.QueriesPath, []string{"../../assets/queries"}) + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &Summary{ + SelectedRemediationsNumber: 0, + ActualRemediationsDoneNumber: 0, + } + + tmpFile := createTempFile(filePath) + s.RemediateFile(tmpFile, tt.args.fix) + + os.Remove(tmpFile) + require.Equal(t, s.ActualRemediationsDoneNumber, tt.actualRemediationsDoneNumber) + + }) + } +} + +func createTempFile(filePath string) string { + // create temporary file + tmpFile := filepath.Join(os.TempDir(), "temporary-remediation"+utils.NextRandom()+filepath.Ext(filePath)) + f, err := os.OpenFile(tmpFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm) + + if err != nil { + f.Close() + return "" + } + + content, err := os.ReadFile(filePath) + if err != nil { + return "" + } + + if _, err = f.Write(content); err != nil { + f.Close() + return "" + } + return tmpFile +} diff --git a/pkg/remediation/remediation_utils.go b/pkg/remediation/utils.go similarity index 98% rename from pkg/remediation/remediation_utils.go rename to pkg/remediation/utils.go index a8a6a2de00d..f8f4dc2fba8 100644 --- a/pkg/remediation/remediation_utils.go +++ b/pkg/remediation/utils.go @@ -1,6 +1,7 @@ package remediation import ( + "fmt" "os" "path/filepath" "regexp" @@ -113,6 +114,7 @@ func removedSimilarityID(results []model.Vulnerability, similarity string) bool result := results[i] if result.SimilarityID == similarity { + fmt.Println(similarity) log.Info().Msgf("failed to remediate '%s'", similarity) return false } diff --git a/pkg/remediation/utils_test.go b/pkg/remediation/utils_test.go new file mode 100644 index 00000000000..46e2f113a1c --- /dev/null +++ b/pkg/remediation/utils_test.go @@ -0,0 +1,113 @@ +package remediation + +import ( + "reflect" + "testing" + + "github.com/stretchr/testify/require" +) + +func Test_GetFixs(t *testing.T) { + + filePath := "assets\\queries\\terraform\\alicloud\\ram_account_password_policy_not_required_symbols\\test\\negative1.tf" + + file1 := &File{ + FilePath: filePath, + Line: 1, + Remediation: "minimum_password_length = 14", + RemediationType: "addition", + SimilarityID: "f282fa13cf5e4ffd4bbb0ee2059f8d0240edcd2ca54b3bb71633145d961de5ce", + } + + file2 := &File{ + FilePath: filePath, + Line: 5, + Remediation: "require_symbols = true", + RemediationType: "replacement", + SimilarityID: "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", + } + + query1 := &Query{ + Files: []File{*file1}, + QueryID: "a9dfec39-a740-4105-bbd6-721ba163c053", + } + + query2 := &Query{ + Files: []File{*file2}, + QueryID: "41a38329-d81b-4be4-aef4-55b2615d3282", + } + + res := &Result{ + Queries: []Query{*query1, *query2}, + } + + replacement := &Remediation{ + Line: 5, + Remediation: "require_symbols = true", + SimilarityID: "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", + QueryID: "41a38329-d81b-4be4-aef4-55b2615d3282", + } + + addition := &Remediation{ + Line: 1, + Remediation: "minimum_password_length = 14", + SimilarityID: "f282fa13cf5e4ffd4bbb0ee2059f8d0240edcd2ca54b3bb71633145d961de5ce", + QueryID: "a9dfec39-a740-4105-bbd6-721ba163c053", + } + + var fix, fix2 Fix + + fix.Replacement = append(fix.Replacement, *replacement) + fix.Addition = append(fix.Addition, *addition) + + want := make(map[string]interface{}) + want[filePath] = fix + + fix2.Replacement = append(fix2.Replacement, *replacement) + want2 := make(map[string]interface{}) + want2[filePath] = fix2 + + type args struct { + res *Result + include []string + } + + tests := []struct { + name string + args args + selectedRemediationsNumber int + want map[string]interface{} + }{ + { + name: "include all similarityID", + args: args{ + res: res, + include: []string{"all"}, + }, + want: want, + selectedRemediationsNumber: 2, + }, + { + name: "include specific similarityID", + args: args{ + res: res, + include: []string{"87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc"}, + }, + want: want2, + selectedRemediationsNumber: 1, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := &Summary{ + SelectedRemediationsNumber: 0, + ActualRemediationsDoneNumber: 0, + } + if got := s.GetFixs(*tt.args.res, tt.args.include); !reflect.DeepEqual(got, tt.want) { + t.Errorf("GetFixs() = %v, want %v", got, tt.want) + } + require.Equal(t, s.SelectedRemediationsNumber, tt.selectedRemediationsNumber) + }) + } +} diff --git a/test/fixtures/kics_auto_remediation/terraform.tf b/test/fixtures/kics_auto_remediation/terraform.tf new file mode 100644 index 00000000000..ff7e89a967e --- /dev/null +++ b/test/fixtures/kics_auto_remediation/terraform.tf @@ -0,0 +1,20 @@ +resource "alicloud_ram_account_password_policy" "corporate1" { + require_lowercase_characters = false + require_uppercase_characters = false + require_numbers = false + require_symbols = false + hard_expiry = true + password_reuse_prevention = 5 + max_login_attempts = 3 +} + +resource "alicloud_ram_account_password_policy" "corporate2" { + minimum_password_length = 14 + require_lowercase_characters = false + require_uppercase_characters = false + require_numbers = false + require_symbols = false + hard_expiry = true + password_reuse_prevention = 5 + max_login_attempts = 3 +} From b3aa98b754736f1ab625d3648f2a717f2dc3ac62 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Thu, 7 Jul 2022 09:37:29 +0100 Subject: [PATCH 03/22] replacement approach change --- .../aws/alb_listening_on_http/query.rego | 6 ++++-- .../dockerfile/add_instead_of_copy/query.rego | 6 ++++-- .../alicloud/cmk_is_unusable/query.rego | 8 ++++--- .../query.rego | 2 +- .../query.rego | 9 ++++---- .../query.rego | 6 ++++-- pkg/engine/vulnerability_builder.go | 2 +- pkg/remediation/remediation.go | 21 +++++++++++++++---- pkg/remediation/remediation_test.go | 7 ++++--- pkg/remediation/utils_test.go | 8 ++++--- 10 files changed, 50 insertions(+), 25 deletions(-) diff --git a/assets/queries/ansible/aws/alb_listening_on_http/query.rego b/assets/queries/ansible/aws/alb_listening_on_http/query.rego index 4027b9d31eb..b3bf8349894 100644 --- a/assets/queries/ansible/aws/alb_listening_on_http/query.rego +++ b/assets/queries/ansible/aws/alb_listening_on_http/query.rego @@ -11,6 +11,8 @@ CxPolicy[result] { applicationLb.listeners[index].Protocol != "HTTPS" + remediation := {"before": applicationLb.listeners[index].Protocol, "after": "HTTPS"} + result := { "documentId": id, "resourceType": modules[m], @@ -19,8 +21,8 @@ CxPolicy[result] { "issueType": "IncorrectValue", "keyExpectedValue": "'aws_elb_application_lb' Protocol should be 'HTTP'", "keyActualValue": "'aws_elb_application_lb' Protocol it's not 'HTTP'", - "remediation": "Protocol: HTTPS", - "remediation_type": "replacement", + "remediation": json.marshal(remediation), + "remediationType": "replacement", } } diff --git a/assets/queries/dockerfile/add_instead_of_copy/query.rego b/assets/queries/dockerfile/add_instead_of_copy/query.rego index 9d706f02338..6274aecbce8 100644 --- a/assets/queries/dockerfile/add_instead_of_copy/query.rego +++ b/assets/queries/dockerfile/add_instead_of_copy/query.rego @@ -8,13 +8,15 @@ CxPolicy[result] { not dockerLib.arrayContains(resource.Value, {".tar", ".tar."}) + remediation := {"before": "ADD", "after": "COPY"} + result := { "documentId": input.document[i].id, "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, resource.Original]), "issueType": "IncorrectValue", "keyExpectedValue": sprintf("'COPY' %s", [resource.Value[0]]), "keyActualValue": sprintf("'ADD' %s", [resource.Value[0]]), - "remediation": sprintf("COPY %s", [concat(" ", resource.Value)]), - "remediation_type": "replacement", + "remediation": json.marshal(remediation), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/cmk_is_unusable/query.rego b/assets/queries/terraform/alicloud/cmk_is_unusable/query.rego index fee1ee1c6dd..e761e95ba95 100644 --- a/assets/queries/terraform/alicloud/cmk_is_unusable/query.rego +++ b/assets/queries/terraform/alicloud/cmk_is_unusable/query.rego @@ -8,6 +8,8 @@ CxPolicy[result] { resource.is_enabled == false + remediation := {"before": "false", "after": "true"} + result := { "documentId": input.document[i].id, "resourceType": "alicloud_kms_key", @@ -17,8 +19,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_kms_key[%s].is_enabled to be set to true", [name]), "keyActualValue": sprintf("alicloud_kms_key[%s].is_enabled is set to false", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_kms_key", name, "is_enabled"], []), - "remediation": "is_enabled = true", - "remediation_type": "replacement", + "remediation": json.marshal(remediation), + "remediationType": "replacement", } } @@ -37,6 +39,6 @@ CxPolicy[result] { "keyActualValue": sprintf("alicloud_kms_key[%s].is_enabled is not set", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_kms_key", name], []), "remediation": "is_enabled = true", - "remediation_type": "addition", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego index 552c5707fd7..25db9fc697f 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego @@ -18,7 +18,7 @@ CxPolicy[result] { "keyActualValue": "'max_password_age' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name], []), "remediation": "max_password_age = 12", - "remediation_type": "addition", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego index bdecdfc9b63..27500ed5d93 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego @@ -7,6 +7,8 @@ CxPolicy[result] { some i resource := input.document[i].resource.alicloud_ram_account_password_policy[name] resource.minimum_password_length < 14 + + remediation := {"before": resource.minimum_password_length, "after": "14"} result := { "documentId": input.document[i].id, @@ -17,9 +19,8 @@ CxPolicy[result] { "keyExpectedValue": "'minimum_password_length' is defined and set to 14 or above", "keyActualValue": "'minimum_password_length' is lower than 14", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "minimum_password_length"], []), - "remediation": "minimum_password_length = 14", - "remediation_type": "replacement", - + "remediation": json.marshal(remediation), + "remediationType": "replacement", } } @@ -39,6 +40,6 @@ CxPolicy[result] { "keyActualValue": "'minimum_password_length' is not difined", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name], []), "remediation": "minimum_password_length = 14", - "remediation_type": "addition", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_symbols/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_symbols/query.rego index 915a0a4d86a..4d385c44f99 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_symbols/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_symbols/query.rego @@ -8,6 +8,8 @@ CxPolicy[result] { resource := document.resource.alicloud_ram_account_password_policy[name] resource["require_symbols"] == false + remediation := {"before": "false", "after": "true"} + result := { "documentId": document.id, "resourceType": "alicloud_ram_account_password_policy", @@ -17,7 +19,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("resource.alicloud_ram_account_password_policy[%s].require_symbols is set to 'true'", [name]), "keyActualValue": sprintf("resource.alicloud_ram_account_password_policy[%s].require_symbols is configured as 'false'", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "require_symbols"], []), - "remediation": "require_symbols = true", - "remediation_type": "replacement", + "remediation": json.marshal(remediation), + "remediationType": "replacement", } } diff --git a/pkg/engine/vulnerability_builder.go b/pkg/engine/vulnerability_builder.go index 5527e5fc12e..b45130b852d 100644 --- a/pkg/engine/vulnerability_builder.go +++ b/pkg/engine/vulnerability_builder.go @@ -145,7 +145,7 @@ var DefaultVulnerabilityBuilder = func(ctx *QueryContext, tracker Tracker, Output: string(output), CloudProvider: getCloudProvider(platform, overrideKey, vObj, &logWithFields), Remediation: PtrStringToString(mustMapKeyToString(vObj, "remediation")), - RemediationType: PtrStringToString(mustMapKeyToString(vObj, "remediation_type")), + RemediationType: PtrStringToString(mustMapKeyToString(vObj, "remediationType")), }, nil } diff --git a/pkg/remediation/remediation.go b/pkg/remediation/remediation.go index 7fcfa050140..5eff1001dc3 100644 --- a/pkg/remediation/remediation.go +++ b/pkg/remediation/remediation.go @@ -1,6 +1,7 @@ package remediation import ( + "encoding/json" "os" "sort" "strings" @@ -78,18 +79,30 @@ func (s *Summary) RemediateFile(filePath string, fix Fix) error { return nil } +type ReplacementInfo struct { + Before string `json:"before"` + After string `json:"after"` +} + func replacement(r Remediation, lines []string) []string { originalLine := lines[r.Line-1] - if strings.Contains(originalLine, r.Remediation) { - log.Info().Msgf("remediation '%s' is already done", r.SimilarityID) + var replacement ReplacementInfo + err := json.Unmarshal([]byte(r.Remediation), &replacement) + + if err != nil || replacement == (ReplacementInfo{}) { return []string{} } - before := getBefore(originalLine) + remediated := strings.Replace(lines[r.Line-1], replacement.Before, replacement.After, 1) + + if strings.Contains(originalLine, remediated) { + log.Info().Msgf("remediation '%s' is already done", r.SimilarityID) + return []string{} + } // replace the original line with remediation - lines[r.Line-1] = before + r.Remediation + lines[r.Line-1] = remediated return lines } diff --git a/pkg/remediation/remediation_test.go b/pkg/remediation/remediation_test.go index 27f24fe888e..50c47b080a3 100644 --- a/pkg/remediation/remediation_test.go +++ b/pkg/remediation/remediation_test.go @@ -13,7 +13,8 @@ import ( ) func Test_RemediateFile(t *testing.T) { - filePath := "../../test/fixtures/kics_auto_remediation/terraform.tf" + + filePath := filepath.Join("..", "..", "test", "fixtures", "kics_auto_remediation", "terraform.tf") type args struct { fix Fix @@ -21,7 +22,7 @@ func Test_RemediateFile(t *testing.T) { replacement := &Remediation{ Line: 5, - Remediation: "require_symbols = true", + Remediation: "{\"after\":\"true\",\"before\":\"false\"}", SimilarityID: "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", QueryID: "41a38329-d81b-4be4-aef4-55b2615d3282", } @@ -41,7 +42,7 @@ func Test_RemediateFile(t *testing.T) { wrongReplacement := &Remediation{ Line: 5, - Remediation: "require_symbols = false", + Remediation: "{\"after\":\"false\",\"before\":\"false\"}", SimilarityID: "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", QueryID: "41a38329-d81b-4be4-aef4-55b2615d3282", } diff --git a/pkg/remediation/utils_test.go b/pkg/remediation/utils_test.go index 46e2f113a1c..664830794e5 100644 --- a/pkg/remediation/utils_test.go +++ b/pkg/remediation/utils_test.go @@ -1,6 +1,7 @@ package remediation import ( + "path/filepath" "reflect" "testing" @@ -9,7 +10,8 @@ import ( func Test_GetFixs(t *testing.T) { - filePath := "assets\\queries\\terraform\\alicloud\\ram_account_password_policy_not_required_symbols\\test\\negative1.tf" + filePath := filepath.Join("assets", "queries", "terraform", "alicloud", + "ram_account_password_policy_not_required_symbols", "test", "negative1.tf") file1 := &File{ FilePath: filePath, @@ -22,7 +24,7 @@ func Test_GetFixs(t *testing.T) { file2 := &File{ FilePath: filePath, Line: 5, - Remediation: "require_symbols = true", + Remediation: "{\"after\":\"true\",\"before\":\"false\"}", RemediationType: "replacement", SimilarityID: "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", } @@ -43,7 +45,7 @@ func Test_GetFixs(t *testing.T) { replacement := &Remediation{ Line: 5, - Remediation: "require_symbols = true", + Remediation: "{\"after\":\"true\",\"before\":\"false\"}", SimilarityID: "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", QueryID: "41a38329-d81b-4be4-aef4-55b2615d3282", } From 610dd6d34b30cb083f1d9ce0e35ddbd4ded7fa07 Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Thu, 7 Jul 2022 09:38:32 +0100 Subject: [PATCH 04/22] alicloud --- .../query.rego | 24 ++++++++++++++++++- .../query.rego | 4 ++++ .../disk_encryption_disabled/query.rego | 8 ++++--- .../query.rego | 4 ++++ .../oss_bucket_lifecycle_disabled/query.rego | 2 ++ .../oss_bucket_logging_disabled/query.rego | 2 ++ .../query.rego | 4 ++++ .../query.rego | 2 ++ .../query.rego | 4 +++- .../query.rego | 4 ++++ .../rds_instance_events_not_logged/query.rego | 4 ++++ .../query.rego | 4 +++- .../query.rego | 2 ++ .../query.rego | 6 +++-- .../ros_stack_retention_disabled/query.rego | 4 ++++ 15 files changed, 70 insertions(+), 8 deletions(-) diff --git a/assets/queries/terraform/alicloud/action_trail_logging_all_regions_disabled/query.rego b/assets/queries/terraform/alicloud/action_trail_logging_all_regions_disabled/query.rego index 5578dc82c93..72eb4c9162c 100644 --- a/assets/queries/terraform/alicloud/action_trail_logging_all_regions_disabled/query.rego +++ b/assets/queries/terraform/alicloud/action_trail_logging_all_regions_disabled/query.rego @@ -6,8 +6,26 @@ import data.generic.terraform as tf_lib CxPolicy[result] { some i resource := input.document[i].resource.alicloud_actiontrail_trail[name] + not common_lib.valid_key(resource, "oss_bucket_name") - possibilities := {"event_rw", "oss_bucket_name", "trail_region"} + + result := { + "documentId": input.document[i].id, + "resourceType": "alicloud_actiontrail_trail", + "resourceName": tf_lib.get_specific_resource_name(resource, "alicloud_actiontrail_trail", name), + "searchKey": sprintf("alicloud_actiontrail_trail[%s]", [name]), + "issueType": "MissingAttribute", + "keyExpectedValue": "oss_bucket_name is set.", + "keyActualValue": "oss_bucket_name is not set.", + "searchLine": common_lib.build_search_line(["resource", "alicloud_actiontrail_trail", name], []), + } +} + +CxPolicy[result] { + some i + resource := input.document[i].resource.alicloud_actiontrail_trail[name] + + possibilities := {"event_rw", "trail_region"} not common_lib.valid_key(resource, possibilities[p]) @@ -20,6 +38,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("'%s' is set.",[possibilities[p]]), "keyActualValue": sprintf("'%s' is not set.",[possibilities[p]]), "searchLine": common_lib.build_search_line(["resource", "alicloud_actiontrail_trail", name], []), + "remediation": sprintf("%s= \"ALL\"", [p]), + "remediation_type": "addition", } } @@ -40,5 +60,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("'%s' is set to All", [p[f]]), "keyActualValue": sprintf("'%s' is not set to All", [p[f]]), "searchLine": common_lib.build_search_line(["resource", "alicloud_actiontrail_trail", name, p[f]], []), + "remediation": sprintf("%s= \"ALL\"", [f]), + "remediation_type": "replacement", } } diff --git a/assets/queries/terraform/alicloud/cs_kubernetes_node_pool_auto_repair_disabled/query.rego b/assets/queries/terraform/alicloud/cs_kubernetes_node_pool_auto_repair_disabled/query.rego index bbb0be32e30..91c8ea7bee6 100644 --- a/assets/queries/terraform/alicloud/cs_kubernetes_node_pool_auto_repair_disabled/query.rego +++ b/assets/queries/terraform/alicloud/cs_kubernetes_node_pool_auto_repair_disabled/query.rego @@ -19,6 +19,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("For the resource alicloud_cs_kubernetes_node_pool[%s] to have 'auto_repair' set to true.", [name]), "keyActualValue": sprintf("The resource alicloud_cs_kubernetes_node_pool[%s] has 'auto_repair' set to false.", [name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_cs_kubernetes_node_pool", name, "management", "auto_repair"], []), + "remediation": "auto_repair = true", + "remediation_type": "replacement", } } @@ -53,5 +55,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("For the resource alicloud_cs_kubernetes_node_pool[%s] to have a 'management' block containing 'auto_repair' set to true.", [name]), "keyActualValue": sprintf("The resource alicloud_cs_kubernetes_node_pool[%s] has a 'management' block but it doesn't contain 'auto_repair' ", [name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_cs_kubernetes_node_pool", name, "management"], []), + "remediation": "auto_repair = true", + "remediation_type": "addition", } } diff --git a/assets/queries/terraform/alicloud/disk_encryption_disabled/query.rego b/assets/queries/terraform/alicloud/disk_encryption_disabled/query.rego index dc7cbf1594a..101e75dfb41 100644 --- a/assets/queries/terraform/alicloud/disk_encryption_disabled/query.rego +++ b/assets/queries/terraform/alicloud/disk_encryption_disabled/query.rego @@ -6,8 +6,7 @@ import data.generic.terraform as tf_lib CxPolicy[result] { resource := input.document[i].resource.alicloud_disk[name] - resource.encrypted == false - + resource.encrypted == false result := { "documentId": input.document[i].id, @@ -18,6 +17,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("[%s] has encryption set to true", [name]), "keyActualValue": sprintf("[%s] has encryption set to false", [name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_disk", name, "encrypted"], []), + "remediation": "encrypted = true", + "remediation_type": "replacement", } } @@ -26,7 +27,6 @@ CxPolicy[result] { resource := input.document[i].resource.alicloud_disk[name] not common_lib.valid_key(resource, "encrypted") not common_lib.valid_key(resource, "snapshot_id") - result := { "documentId": input.document[i].id, @@ -37,6 +37,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("[%s] has encryption enabled",[name]), "keyActualValue": sprintf("[%s] does not have encryption enabled",[name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_disk", name], []), + "remediation": "encrypted = true", + "remediation_type": "addition", } } diff --git a/assets/queries/terraform/alicloud/launch_template_is_not_encrypted/query.rego b/assets/queries/terraform/alicloud/launch_template_is_not_encrypted/query.rego index 164e077d514..92d199fbc3b 100644 --- a/assets/queries/terraform/alicloud/launch_template_is_not_encrypted/query.rego +++ b/assets/queries/terraform/alicloud/launch_template_is_not_encrypted/query.rego @@ -16,6 +16,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_launch_template[%s].encrypted to be true", [name]), "keyActualValue": sprintf("alicloud_launch_template[%s].encrypted is false", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_launch_template", name, "encrypted"], []), + "remediation": "encrypted = true", + "remediation_type": "replacement", } } @@ -32,5 +34,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_launch_template[%s] 'encrypted' should be defined and set to true", [name]), "keyActualValue": sprintf("alicloud_launch_template[%s] 'encrypted' argument is not defined", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_launch_template", name], []), + "remediation": "encrypted = true", + "remediation_type": "addition", } } diff --git a/assets/queries/terraform/alicloud/oss_bucket_lifecycle_disabled/query.rego b/assets/queries/terraform/alicloud/oss_bucket_lifecycle_disabled/query.rego index 4b6e22faaf0..d54eb0ed8f5 100644 --- a/assets/queries/terraform/alicloud/oss_bucket_lifecycle_disabled/query.rego +++ b/assets/queries/terraform/alicloud/oss_bucket_lifecycle_disabled/query.rego @@ -18,6 +18,8 @@ CxPolicy[result] { "keyExpectedValue": "'lifecycle_rule' is set and enabled", "keyActualValue": "'lifecycle_rule' is set but disabled", "searchLine":common_lib.build_search_line(["resource", "alicloud_oss_bucket", name, "lifecycle_rule", "enabled"], []), + "remediation": "enabled = true", + "remediation_type": "replacement", } } diff --git a/assets/queries/terraform/alicloud/oss_bucket_logging_disabled/query.rego b/assets/queries/terraform/alicloud/oss_bucket_logging_disabled/query.rego index d2419789a7a..15520254463 100644 --- a/assets/queries/terraform/alicloud/oss_bucket_logging_disabled/query.rego +++ b/assets/queries/terraform/alicloud/oss_bucket_logging_disabled/query.rego @@ -34,5 +34,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("%s 'logging_isenable' argument should be set to true",[name]), "keyActualValue": sprintf("%s 'logging_isenable' argument is set to false",[name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_oss_bucket", name, "logging_isenable"], []), + "remediation": "logging_isenable = true", + "remediation_type": "replacement", } } diff --git a/assets/queries/terraform/alicloud/oss_bucket_transfer_acceleration_disabled/query.rego b/assets/queries/terraform/alicloud/oss_bucket_transfer_acceleration_disabled/query.rego index 40ca6d46d95..6c88c9b0d71 100644 --- a/assets/queries/terraform/alicloud/oss_bucket_transfer_acceleration_disabled/query.rego +++ b/assets/queries/terraform/alicloud/oss_bucket_transfer_acceleration_disabled/query.rego @@ -18,6 +18,8 @@ CxPolicy[result] { "keyExpectedValue": "'transfer_acceleration.enabled' is defined and set to true", "keyActualValue": "'transfer_acceleration.enabled' is false", "searchLine": common_lib.build_search_line(["resource", "alicloud_oss_bucket", name, "transfer_acceleration", "enabled"], []), + "remediation": "enabled = true", + "remediation_type": "replacement", } } @@ -36,5 +38,7 @@ CxPolicy[result] { "keyExpectedValue": "'transfer_acceleration.enabled' is defined and set to true", "keyActualValue": "'transfer_acceleration' is missing", "searchLine": common_lib.build_search_line(["resource", "alicloud_oss_bucket", name], []), + "remediation": "transfer_acceleration{\n\t\tenabled = true\n\t}", + "remediation_type": "addition", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_numbers/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_numbers/query.rego index 85e32916612..ef090b34166 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_numbers/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_numbers/query.rego @@ -17,5 +17,7 @@ CxPolicy[result] { "keyExpectedValue": "'require_numbers' is defined and set to true", "keyActualValue": "'require_numbers' is false", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "require_numbers"], []), + "remediation": "require_numbers = true", + "remediation_type": "replacement", } } diff --git a/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_uppercase_character/query.rego b/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_uppercase_character/query.rego index c90f9db9ebd..853dce8b108 100644 --- a/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_uppercase_character/query.rego +++ b/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_uppercase_character/query.rego @@ -16,6 +16,8 @@ CxPolicy[result] { "issueType": "IncorrectValue", "keyExpectedValue": "'require_uppercase_characters' is defined and set to true", "keyActualValue": "'require_uppercase_characters' is false", - "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "require_uppercase_characters"], []), + "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "require_uppercase_characters"], []), + "remediation": "require_uppercase_characters = true", + "remediation_type": "replacement", } } diff --git a/assets/queries/terraform/alicloud/ram_security_preference_not_enforce_mfa/query.rego b/assets/queries/terraform/alicloud/ram_security_preference_not_enforce_mfa/query.rego index b13e861b264..5ff0cda97a7 100644 --- a/assets/queries/terraform/alicloud/ram_security_preference_not_enforce_mfa/query.rego +++ b/assets/queries/terraform/alicloud/ram_security_preference_not_enforce_mfa/query.rego @@ -44,6 +44,8 @@ CxPolicy[result] { "keyExpectedValue": "'enforce_mfa_for_login' should be defined and set to true", "keyActualValue": "'enforce_mfa_for_login' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_security_preference", name], []), + "remediation": "enforce_mfa_for_login = true", + "remediation_type": "addition", } } @@ -61,5 +63,7 @@ CxPolicy[result] { "keyExpectedValue": "'enforce_mfa_for_login' should be set to true", "keyActualValue": "'enforce_mfa_for_login' is set to 'false'", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_security_preference", name, "enforce_mfa_for_login" ], []), + "remediation": "enforce_mfa_for_login = true", + "remediation_type": "replacement", } } diff --git a/assets/queries/terraform/alicloud/rds_instance_events_not_logged/query.rego b/assets/queries/terraform/alicloud/rds_instance_events_not_logged/query.rego index 73af7aa204d..b8b78599f46 100644 --- a/assets/queries/terraform/alicloud/rds_instance_events_not_logged/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_events_not_logged/query.rego @@ -22,6 +22,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("'%s' parameter value should be 'true'", [log]), "keyActualValue": sprintf("'%s' parameter is not defined", [log]), "searchLine": common_lib.build_search_line(["resource", "alicloud_log_audit", name, "variable_map"], []), + "remediation": sprintf("%s = true",[log]), + "remediation_type": "addition", } } @@ -40,6 +42,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("'%s' parameter value should be 'true'", [log]), "keyActualValue": sprintf("'%s' parameter value is 'false'", [log]), "searchLine": common_lib.build_search_line(["resource", "alicloud_log_audit", name, "variable_map", log], []), + "remediation": sprintf("%s = true",[log]), + "remediation_type": "replacement", } } diff --git a/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/query.rego b/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/query.rego index 4f9aee356a2..5672b3d277a 100644 --- a/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/query.rego @@ -18,6 +18,8 @@ CxPolicy[result] { "keyExpectedValue": "'log_connections' parameter value should be 'ON'", "keyActualValue": "'log_connections' parameter value is 'OFF'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name, "parameters", parameter, "value"], []), + "remediation": "value = \"ON\"", + "remediation_type": "replacement", } } @@ -30,7 +32,7 @@ CxPolicy[result] { "documentId": input.document[i].id, "resourceType": "alicloud_db_instance", "resourceName": tf_lib.get_resource_name(resource, name), - "searchKey": sprintf("alicloud_db_instance[%s]]", [name]), + "searchKey": sprintf("alicloud_db_instance[%s]", [name]), "issueType": "MissingAttribute", "keyExpectedValue": "'log_connections' parameter is defined value should be 'ON'", "keyActualValue": "'log_connections' parameter is not defined", diff --git a/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/query.rego b/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/query.rego index 95743bbb4f1..eaa3fbb187b 100644 --- a/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/query.rego @@ -18,6 +18,8 @@ CxPolicy[result] { "keyExpectedValue": "'log_disconnections' parameter value should be 'ON'", "keyActualValue": "'log_disconnections' parameter value is 'OFF'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name, "parameters", parameter, "value"], []), + "remediation": "value = \"ON\"", + "remediation_type": "replacement", } } diff --git a/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/query.rego b/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/query.rego index 5df1ab5080f..af250688d2f 100644 --- a/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/query.rego @@ -18,13 +18,15 @@ CxPolicy[result] { "keyExpectedValue": "'log_duration' parameter value should be 'ON'", "keyActualValue": "'log_duration' parameter value is 'OFF'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name, "parameters", parameter, "value"], []), + "remediation": "value = \"ON\"", + "remediation_type": "replacement", } } CxPolicy[result] { some i resource := input.document[i].resource.alicloud_db_instance[name] - not has_log_disconn(resource) + not has_log_duration(resource) result := { "documentId": input.document[i].id, @@ -38,7 +40,7 @@ CxPolicy[result] { } } -has_log_disconn(resource){ +has_log_duration(resource){ parameter := resource.parameters[j] parameter.name == "log_duration" } diff --git a/assets/queries/terraform/alicloud/ros_stack_retention_disabled/query.rego b/assets/queries/terraform/alicloud/ros_stack_retention_disabled/query.rego index cc76b1b072c..5809f9a6f7b 100644 --- a/assets/queries/terraform/alicloud/ros_stack_retention_disabled/query.rego +++ b/assets/queries/terraform/alicloud/ros_stack_retention_disabled/query.rego @@ -17,6 +17,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_ros_stack_instance[%s].retain_stacks should be defined and not null", [name]), "keyActualValue": sprintf("alicloud_ros_stack_instance[%s].retain_stacks is undefined", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_ros_stack_instance", name], []), + "remediation": "retain_stacks = true", + "remediation_type": "addition", } } @@ -34,5 +36,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_ros_stack_instance[%s].retain_stacks should be true ", [name]), "keyActualValue": sprintf("alicloud_ros_stack_instance[%s].retain_stacks is false", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_ros_stack_instance", name, "retain_stacks"], []), + "remediation": "retain_stacks = true", + "remediation_type": "replacement", } } From dd76d59f3e45493852d0eaf3915ef5eed83d9779 Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Thu, 7 Jul 2022 12:09:00 +0100 Subject: [PATCH 05/22] QUERIES THAT VERIFY A FIELD SET TO FALSE --- .../query.rego | 8 ++--- .../query.rego | 11 +++--- .../disk_encryption_disabled/query.rego | 9 +++-- .../query.rego | 9 +++-- .../oss_bucket_lifecycle_disabled/query.rego | 7 ++-- .../oss_bucket_logging_disabled/query.rego | 7 ++-- .../query.rego | 9 +++-- .../query.rego | 7 ++-- .../query.rego | 7 +++- .../query.rego | 7 ++-- .../query.rego | 9 +++-- .../rds_instance_events_not_logged/query.rego | 9 +++-- .../query.rego | 35 +++++++++++++++--- .../test/positive3.tf | 6 ++++ .../test/positive_expected_result.json | 10 ++++-- .../query.rego | 36 ++++++++++++++++--- .../test/positive3.tf | 6 ++++ .../test/positive_expected_result.json | 10 ++++-- .../query.rego | 36 ++++++++++++++++--- .../test/positive3.tf | 6 ++++ .../test/positive_expected_result.json | 8 ++++- .../ros_stack_retention_disabled/query.rego | 9 +++-- res/positive1.tf | 4 +++ res/positive2.tf | 9 +++++ 24 files changed, 222 insertions(+), 52 deletions(-) create mode 100644 assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/test/positive3.tf create mode 100644 assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/test/positive3.tf create mode 100644 assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/test/positive3.tf create mode 100644 res/positive1.tf create mode 100644 res/positive2.tf diff --git a/assets/queries/terraform/alicloud/action_trail_logging_all_regions_disabled/query.rego b/assets/queries/terraform/alicloud/action_trail_logging_all_regions_disabled/query.rego index 72eb4c9162c..4e2b161aece 100644 --- a/assets/queries/terraform/alicloud/action_trail_logging_all_regions_disabled/query.rego +++ b/assets/queries/terraform/alicloud/action_trail_logging_all_regions_disabled/query.rego @@ -39,7 +39,7 @@ CxPolicy[result] { "keyActualValue": sprintf("'%s' is not set.",[possibilities[p]]), "searchLine": common_lib.build_search_line(["resource", "alicloud_actiontrail_trail", name], []), "remediation": sprintf("%s= \"ALL\"", [p]), - "remediation_type": "addition", + "remediationType": "addition", } } @@ -50,7 +50,7 @@ CxPolicy[result] { p := {"event_rw", "trail_region"} resource[p[f]] != "All" - + remediation := {"before":resource[p[f]] , "after": "All" } result := { "documentId": input.document[i].id, "resourceType": "alicloud_actiontrail_trail", @@ -60,7 +60,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("'%s' is set to All", [p[f]]), "keyActualValue": sprintf("'%s' is not set to All", [p[f]]), "searchLine": common_lib.build_search_line(["resource", "alicloud_actiontrail_trail", name, p[f]], []), - "remediation": sprintf("%s= \"ALL\"", [f]), - "remediation_type": "replacement", + "remediation": json.marshal(remediation), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/cs_kubernetes_node_pool_auto_repair_disabled/query.rego b/assets/queries/terraform/alicloud/cs_kubernetes_node_pool_auto_repair_disabled/query.rego index 91c8ea7bee6..47f9e8e8b01 100644 --- a/assets/queries/terraform/alicloud/cs_kubernetes_node_pool_auto_repair_disabled/query.rego +++ b/assets/queries/terraform/alicloud/cs_kubernetes_node_pool_auto_repair_disabled/query.rego @@ -9,7 +9,7 @@ CxPolicy[result] { auto_repair := resource.management.auto_repair auto_repair == false - + result := { "documentId": input.document[i].id, "resourceType": "alicloud_cs_kubernetes_node_pool", @@ -19,8 +19,11 @@ CxPolicy[result] { "keyExpectedValue": sprintf("For the resource alicloud_cs_kubernetes_node_pool[%s] to have 'auto_repair' set to true.", [name]), "keyActualValue": sprintf("The resource alicloud_cs_kubernetes_node_pool[%s] has 'auto_repair' set to false.", [name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_cs_kubernetes_node_pool", name, "management", "auto_repair"], []), - "remediation": "auto_repair = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } @@ -56,6 +59,6 @@ CxPolicy[result] { "keyActualValue": sprintf("The resource alicloud_cs_kubernetes_node_pool[%s] has a 'management' block but it doesn't contain 'auto_repair' ", [name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_cs_kubernetes_node_pool", name, "management"], []), "remediation": "auto_repair = true", - "remediation_type": "addition", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/disk_encryption_disabled/query.rego b/assets/queries/terraform/alicloud/disk_encryption_disabled/query.rego index 101e75dfb41..e3d81215f83 100644 --- a/assets/queries/terraform/alicloud/disk_encryption_disabled/query.rego +++ b/assets/queries/terraform/alicloud/disk_encryption_disabled/query.rego @@ -17,8 +17,11 @@ CxPolicy[result] { "keyExpectedValue": sprintf("[%s] has encryption set to true", [name]), "keyActualValue": sprintf("[%s] has encryption set to false", [name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_disk", name, "encrypted"], []), - "remediation": "encrypted = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } @@ -38,7 +41,7 @@ CxPolicy[result] { "keyActualValue": sprintf("[%s] does not have encryption enabled",[name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_disk", name], []), "remediation": "encrypted = true", - "remediation_type": "addition", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/launch_template_is_not_encrypted/query.rego b/assets/queries/terraform/alicloud/launch_template_is_not_encrypted/query.rego index 92d199fbc3b..4ab990f431c 100644 --- a/assets/queries/terraform/alicloud/launch_template_is_not_encrypted/query.rego +++ b/assets/queries/terraform/alicloud/launch_template_is_not_encrypted/query.rego @@ -16,8 +16,11 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_launch_template[%s].encrypted to be true", [name]), "keyActualValue": sprintf("alicloud_launch_template[%s].encrypted is false", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_launch_template", name, "encrypted"], []), - "remediation": "encrypted = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } @@ -35,6 +38,6 @@ CxPolicy[result] { "keyActualValue": sprintf("alicloud_launch_template[%s] 'encrypted' argument is not defined", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_launch_template", name], []), "remediation": "encrypted = true", - "remediation_type": "addition", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/oss_bucket_lifecycle_disabled/query.rego b/assets/queries/terraform/alicloud/oss_bucket_lifecycle_disabled/query.rego index d54eb0ed8f5..2170aa1a4a9 100644 --- a/assets/queries/terraform/alicloud/oss_bucket_lifecycle_disabled/query.rego +++ b/assets/queries/terraform/alicloud/oss_bucket_lifecycle_disabled/query.rego @@ -18,8 +18,11 @@ CxPolicy[result] { "keyExpectedValue": "'lifecycle_rule' is set and enabled", "keyActualValue": "'lifecycle_rule' is set but disabled", "searchLine":common_lib.build_search_line(["resource", "alicloud_oss_bucket", name, "lifecycle_rule", "enabled"], []), - "remediation": "enabled = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/oss_bucket_logging_disabled/query.rego b/assets/queries/terraform/alicloud/oss_bucket_logging_disabled/query.rego index 15520254463..b980753e03f 100644 --- a/assets/queries/terraform/alicloud/oss_bucket_logging_disabled/query.rego +++ b/assets/queries/terraform/alicloud/oss_bucket_logging_disabled/query.rego @@ -34,7 +34,10 @@ CxPolicy[result] { "keyExpectedValue": sprintf("%s 'logging_isenable' argument should be set to true",[name]), "keyActualValue": sprintf("%s 'logging_isenable' argument is set to false",[name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_oss_bucket", name, "logging_isenable"], []), - "remediation": "logging_isenable = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/oss_bucket_transfer_acceleration_disabled/query.rego b/assets/queries/terraform/alicloud/oss_bucket_transfer_acceleration_disabled/query.rego index 6c88c9b0d71..40e8262ea3c 100644 --- a/assets/queries/terraform/alicloud/oss_bucket_transfer_acceleration_disabled/query.rego +++ b/assets/queries/terraform/alicloud/oss_bucket_transfer_acceleration_disabled/query.rego @@ -18,8 +18,11 @@ CxPolicy[result] { "keyExpectedValue": "'transfer_acceleration.enabled' is defined and set to true", "keyActualValue": "'transfer_acceleration.enabled' is false", "searchLine": common_lib.build_search_line(["resource", "alicloud_oss_bucket", name, "transfer_acceleration", "enabled"], []), - "remediation": "enabled = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } @@ -39,6 +42,6 @@ CxPolicy[result] { "keyActualValue": "'transfer_acceleration' is missing", "searchLine": common_lib.build_search_line(["resource", "alicloud_oss_bucket", name], []), "remediation": "transfer_acceleration{\n\t\tenabled = true\n\t}", - "remediation_type": "addition", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_numbers/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_numbers/query.rego index ef090b34166..1c663788690 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_numbers/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_numbers/query.rego @@ -17,7 +17,10 @@ CxPolicy[result] { "keyExpectedValue": "'require_numbers' is defined and set to true", "keyActualValue": "'require_numbers' is false", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "require_numbers"], []), - "remediation": "require_numbers = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_lowercase_character/query.rego b/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_lowercase_character/query.rego index 72d2733336c..9f6e401d351 100644 --- a/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_lowercase_character/query.rego +++ b/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_lowercase_character/query.rego @@ -16,6 +16,11 @@ CxPolicy[result] { "issueType": "IncorrectValue", "keyExpectedValue": "'require_lowercase_characters' is defined and set to true", "keyActualValue": "'require_lowercase_characters' is false", - "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "require_lowercase_characters"], []), + "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "require_lowercase_characters"], []), + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement" } } diff --git a/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_uppercase_character/query.rego b/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_uppercase_character/query.rego index 853dce8b108..e022f365a2c 100644 --- a/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_uppercase_character/query.rego +++ b/assets/queries/terraform/alicloud/ram_password_security_policy_not_require_at_least_one_uppercase_character/query.rego @@ -17,7 +17,10 @@ CxPolicy[result] { "keyExpectedValue": "'require_uppercase_characters' is defined and set to true", "keyActualValue": "'require_uppercase_characters' is false", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "require_uppercase_characters"], []), - "remediation": "require_uppercase_characters = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/ram_security_preference_not_enforce_mfa/query.rego b/assets/queries/terraform/alicloud/ram_security_preference_not_enforce_mfa/query.rego index 5ff0cda97a7..e0976c4c4db 100644 --- a/assets/queries/terraform/alicloud/ram_security_preference_not_enforce_mfa/query.rego +++ b/assets/queries/terraform/alicloud/ram_security_preference_not_enforce_mfa/query.rego @@ -45,7 +45,7 @@ CxPolicy[result] { "keyActualValue": "'enforce_mfa_for_login' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_security_preference", name], []), "remediation": "enforce_mfa_for_login = true", - "remediation_type": "addition", + "remediationType": "addition", } } @@ -63,7 +63,10 @@ CxPolicy[result] { "keyExpectedValue": "'enforce_mfa_for_login' should be set to true", "keyActualValue": "'enforce_mfa_for_login' is set to 'false'", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_security_preference", name, "enforce_mfa_for_login" ], []), - "remediation": "enforce_mfa_for_login = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/rds_instance_events_not_logged/query.rego b/assets/queries/terraform/alicloud/rds_instance_events_not_logged/query.rego index b8b78599f46..527b327272f 100644 --- a/assets/queries/terraform/alicloud/rds_instance_events_not_logged/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_events_not_logged/query.rego @@ -23,7 +23,7 @@ CxPolicy[result] { "keyActualValue": sprintf("'%s' parameter is not defined", [log]), "searchLine": common_lib.build_search_line(["resource", "alicloud_log_audit", name, "variable_map"], []), "remediation": sprintf("%s = true",[log]), - "remediation_type": "addition", + "remediationType": "addition", } } @@ -42,8 +42,11 @@ CxPolicy[result] { "keyExpectedValue": sprintf("'%s' parameter value should be 'true'", [log]), "keyActualValue": sprintf("'%s' parameter value is 'false'", [log]), "searchLine": common_lib.build_search_line(["resource", "alicloud_log_audit", name, "variable_map", log], []), - "remediation": sprintf("%s = true",[log]), - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/query.rego b/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/query.rego index 5672b3d277a..221a9fe1522 100644 --- a/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/query.rego @@ -18,25 +18,34 @@ CxPolicy[result] { "keyExpectedValue": "'log_connections' parameter value should be 'ON'", "keyActualValue": "'log_connections' parameter value is 'OFF'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name, "parameters", parameter, "value"], []), - "remediation": "value = \"ON\"", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "OFF", + "after": "ON" + }), + "remediationType": "replacement", } } CxPolicy[result] { some i resource := input.document[i].resource.alicloud_db_instance[name] + common_lib.valid_key(resource, "parameters") not has_log_conn(resource) result := { "documentId": input.document[i].id, "resourceType": "alicloud_db_instance", "resourceName": tf_lib.get_resource_name(resource, name), - "searchKey": sprintf("alicloud_db_instance[%s]", [name]), + "searchKey": sprintf("alicloud_db_instance[%s].parameters", [name]), "issueType": "MissingAttribute", "keyExpectedValue": "'log_connections' parameter is defined value should be 'ON'", "keyActualValue": "'log_connections' parameter is not defined", - "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], ["parameters"]), + "remediation": json.marshal({ + "before": "[", + "after": "[{\n\t\tname = \"log_connections\"\n\t\tvalue = \"ON\"\n\t}," + }), + "remediationType": "replacement", } } @@ -45,3 +54,21 @@ has_log_conn(resource){ parameter.name == "log_connections" } +CxPolicy[result] { + some i + resource := input.document[i].resource.alicloud_db_instance[name] + not common_lib.valid_key(resource, "parameters") + + result := { + "documentId": input.document[i].id, + "resourceType": "alicloud_db_instance", + "resourceName": tf_lib.get_resource_name(resource, name), + "searchKey": sprintf("alicloud_db_instance[%s]", [name]), + "issueType": "MissingAttribute", + "keyExpectedValue": "'log_connections' parameter is defined value should be 'ON' in parameters array", + "keyActualValue": "'log_connections' parameter is not defined in parameters array", + "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "remediation": "parameters = [{\n\t\tname = \"log_connections\"\n\t\tvalue = \"ON\"\n\t}]", + "remediationType": "addition", + } +} diff --git a/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/test/positive3.tf b/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/test/positive3.tf new file mode 100644 index 00000000000..99fd38c3118 --- /dev/null +++ b/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/test/positive3.tf @@ -0,0 +1,6 @@ +resource "alicloud_db_instance" "default" { + engine = "MySQL" + engine_version = "5.6" + db_instance_class = "rds.mysql.t1.small" + db_instance_storage = "10" +} diff --git a/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/test/positive_expected_result.json b/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/test/positive_expected_result.json index 396c8d3265b..5646d5b8736 100644 --- a/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/test/positive_expected_result.json +++ b/assets/queries/terraform/alicloud/rds_instance_log_connections_disabled/test/positive_expected_result.json @@ -2,7 +2,7 @@ { "queryName": "RDS Instance Log Connections Disabled", "severity": "LOW", - "line": 1, + "line": 6, "fileName": "positive1.tf" }, { @@ -10,5 +10,11 @@ "severity": "LOW", "line": 14, "fileName": "positive2.tf" - } + }, + { + "queryName": "RDS Instance Log Connections Disabled", + "severity": "LOW", + "line": 1, + "fileName": "positive3.tf" + } ] diff --git a/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/query.rego b/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/query.rego index eaa3fbb187b..ef78ec76c82 100644 --- a/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/query.rego @@ -18,25 +18,34 @@ CxPolicy[result] { "keyExpectedValue": "'log_disconnections' parameter value should be 'ON'", "keyActualValue": "'log_disconnections' parameter value is 'OFF'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name, "parameters", parameter, "value"], []), - "remediation": "value = \"ON\"", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "OFF", + "after": "ON" + }), + "remediationType": "replacement", } } CxPolicy[result] { some i resource := input.document[i].resource.alicloud_db_instance[name] + common_lib.valid_key(resource, "parameters") not has_log_disconn(resource) result := { "documentId": input.document[i].id, "resourceType": "alicloud_db_instance", "resourceName": tf_lib.get_resource_name(resource, name), - "searchKey": sprintf("alicloud_db_instance[%s]]", [name]), + "searchKey": sprintf("alicloud_db_instance[%s].parameters", [name]), "issueType": "MissingAttribute", "keyExpectedValue": "'log_disconnections' parameter is defined and value should be 'ON'", "keyActualValue": "'log_disconnections' parameter is not defined", - "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], ["parameters"]), + "remediation": json.marshal({ + "before": "[", + "after": "[{\n\t\tname = \"log_disconnections\"\n\t\tvalue = \"ON\"\n\t}," + }), + "remediationType": "replacement", } } @@ -44,3 +53,22 @@ has_log_disconn(resource){ parameter := resource.parameters[j] parameter.name == "log_disconnections" } + +CxPolicy[result] { + some i + resource := input.document[i].resource.alicloud_db_instance[name] + not common_lib.valid_key(resource, "parameters") + + result := { + "documentId": input.document[i].id, + "resourceType": "alicloud_db_instance", + "resourceName": tf_lib.get_resource_name(resource, name), + "searchKey": sprintf("alicloud_db_instance[%s]]", [name]), + "issueType": "MissingAttribute", + "keyExpectedValue": "'log_disconnections' parameter is defined and value should be 'ON' in parametes array", + "keyActualValue": "'log_disconnections' parameter is not defined in parametes array", + "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "remediation": "parameters = [{\n\t\tname = \"log_disconnections\"\n\t\tvalue = \"ON\"\n\t}]", + "remediationType": "addition", + } +} diff --git a/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/test/positive3.tf b/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/test/positive3.tf new file mode 100644 index 00000000000..99fd38c3118 --- /dev/null +++ b/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/test/positive3.tf @@ -0,0 +1,6 @@ +resource "alicloud_db_instance" "default" { + engine = "MySQL" + engine_version = "5.6" + db_instance_class = "rds.mysql.t1.small" + db_instance_storage = "10" +} diff --git a/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/test/positive_expected_result.json b/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/test/positive_expected_result.json index 71250c921be..61953fd824d 100644 --- a/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/test/positive_expected_result.json +++ b/assets/queries/terraform/alicloud/rds_instance_log_disconenctions_disabled/test/positive_expected_result.json @@ -8,7 +8,13 @@ { "queryName": "RDS Instance Log Disconnections Disabled", "severity": "LOW", - "line": 1, + "line": 6, "fileName": "positive2.tf" - } + }, + { + "queryName": "RDS Instance Log Disconnections Disabled", + "severity": "LOW", + "line": 1, + "fileName": "positive3.tf" + } ] diff --git a/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/query.rego b/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/query.rego index af250688d2f..553e3c0dbac 100644 --- a/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/query.rego @@ -18,25 +18,34 @@ CxPolicy[result] { "keyExpectedValue": "'log_duration' parameter value should be 'ON'", "keyActualValue": "'log_duration' parameter value is 'OFF'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name, "parameters", parameter, "value"], []), - "remediation": "value = \"ON\"", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "OFF", + "after": "ON" + }), + "remediationType": "replacement", } } CxPolicy[result] { some i resource := input.document[i].resource.alicloud_db_instance[name] + common_lib.valid_key(resource, "parameters") not has_log_duration(resource) result := { "documentId": input.document[i].id, "resourceType": "alicloud_db_instance", "resourceName": tf_lib.get_resource_name(resource, name), - "searchKey": sprintf("alicloud_db_instance[%s]]", [name]), + "searchKey": sprintf("alicloud_db_instance[%s].parameters", [name]), "issueType": "MissingAttribute", "keyExpectedValue": "'log_duration' parameter is defined and value should be 'ON'", "keyActualValue": "'log_duration' parameter is not defined", - "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], ["parameters"]), + "remediation": json.marshal({ + "before": "[", + "after": "[{\n\t\tname = \"log_duration\"\n\t\tvalue = \"ON\"\n\t}," + }), + "remediationType": "replacement", } } @@ -44,3 +53,22 @@ has_log_duration(resource){ parameter := resource.parameters[j] parameter.name == "log_duration" } + +CxPolicy[result] { + some i + resource := input.document[i].resource.alicloud_db_instance[name] + not common_lib.valid_key(resource, "parameters") + + result := { + "documentId": input.document[i].id, + "resourceType": "alicloud_db_instance", + "resourceName": tf_lib.get_resource_name(resource, name), + "searchKey": sprintf("alicloud_db_instance[%s]]", [name]), + "issueType": "MissingAttribute", + "keyExpectedValue": "'log_duration' parameter is defined and value should be 'ON' in parameters array", + "keyActualValue": "'log_duration' parameter is not defined in parameters array", + "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "remediation": "parameters = [{\n\t\tname = \"log_duration\"\n\t\tvalue = \"ON\"\n\t}]", + "remediationType": "addition", + } +} diff --git a/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/test/positive3.tf b/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/test/positive3.tf new file mode 100644 index 00000000000..99fd38c3118 --- /dev/null +++ b/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/test/positive3.tf @@ -0,0 +1,6 @@ +resource "alicloud_db_instance" "default" { + engine = "MySQL" + engine_version = "5.6" + db_instance_class = "rds.mysql.t1.small" + db_instance_storage = "10" +} diff --git a/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/test/positive_expected_result.json b/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/test/positive_expected_result.json index 80846a644ad..3773fc7ad7e 100644 --- a/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/test/positive_expected_result.json +++ b/assets/queries/terraform/alicloud/rds_instance_log_duration_disabled/test/positive_expected_result.json @@ -8,7 +8,13 @@ { "queryName": "RDS Instance Log Duration Disabled", "severity": "LOW", - "line": 1, + "line": 6, "fileName": "positive2.tf" + }, + { + "queryName": "RDS Instance Log Duration Disabled", + "severity": "LOW", + "line": 1, + "fileName": "positive3.tf" } ] diff --git a/assets/queries/terraform/alicloud/ros_stack_retention_disabled/query.rego b/assets/queries/terraform/alicloud/ros_stack_retention_disabled/query.rego index 5809f9a6f7b..7e8a1913ffd 100644 --- a/assets/queries/terraform/alicloud/ros_stack_retention_disabled/query.rego +++ b/assets/queries/terraform/alicloud/ros_stack_retention_disabled/query.rego @@ -18,7 +18,7 @@ CxPolicy[result] { "keyActualValue": sprintf("alicloud_ros_stack_instance[%s].retain_stacks is undefined", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_ros_stack_instance", name], []), "remediation": "retain_stacks = true", - "remediation_type": "addition", + "remediationType": "addition", } } @@ -36,7 +36,10 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_ros_stack_instance[%s].retain_stacks should be true ", [name]), "keyActualValue": sprintf("alicloud_ros_stack_instance[%s].retain_stacks is false", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_ros_stack_instance", name, "retain_stacks"], []), - "remediation": "retain_stacks = true", - "remediation_type": "replacement", + "remediation": json.marshal({ + "before": "false", + "after": "true" + }), + "remediationType": "replacement", } } diff --git a/res/positive1.tf b/res/positive1.tf new file mode 100644 index 00000000000..a48eed94674 --- /dev/null +++ b/res/positive1.tf @@ -0,0 +1,4 @@ +resource "alicloud_oss_bucket" "bucket_logging2" { + bucket = "bucket-170309-acl" + acl = "public-read" +} diff --git a/res/positive2.tf b/res/positive2.tf new file mode 100644 index 00000000000..b7a98bb3564 --- /dev/null +++ b/res/positive2.tf @@ -0,0 +1,9 @@ +resource "alicloud_oss_bucket" "bucket_logging1" { + bucket = "bucket-170309-logging" + logging_isenable = true + + logging { + target_bucket = alicloud_oss_bucket.bucket-target.id + target_prefix = "log/" + } +} From 54799d541ca8b0c5da2f44e08cfb62f395900d69 Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Thu, 7 Jul 2022 14:57:17 +0100 Subject: [PATCH 06/22] UNRECOMMENDED VALUE --- .../query.rego | 17 +++++++++++------ .../test/positive_expected_result.json | 4 ++-- .../alicloud/alb_listening_on_http/query.rego | 5 +++++ .../query.rego | 10 ++++++++++ .../high_kms_key_rotation_period/query.rego | 12 ++++++++++++ .../query.rego | 7 +++++++ .../nas_file_system_not_encrypted/query.rego | 7 +++++++ .../nas_file_system_without_kms/query.rego | 7 +++++++ .../oss_bucket_public_access_enabled/query.rego | 5 +++++ .../oss_bucket_versioning_disabled/query.rego | 7 +++++++ .../query.rego | 6 +++++- .../query.rego | 11 ++++++++++- .../query.rego | 9 +++++++-- .../query.rego | 14 ++++++++++++++ .../rds_instance_ssl_action_disabled/query.rego | 7 +++++++ .../rds_instance_tde_status_disable/query.rego | 14 ++++++++++++++ res/positive1.tf | 4 ---- res/positive2.tf | 9 --------- 18 files changed, 130 insertions(+), 25 deletions(-) delete mode 100644 res/positive1.tf delete mode 100644 res/positive2.tf diff --git a/assets/queries/terraform/alicloud/actiontrail_trail_oss_bucket_is_publicly_accessible/query.rego b/assets/queries/terraform/alicloud/actiontrail_trail_oss_bucket_is_publicly_accessible/query.rego index 6aec0f56313..f94123a6551 100644 --- a/assets/queries/terraform/alicloud/actiontrail_trail_oss_bucket_is_publicly_accessible/query.rego +++ b/assets/queries/terraform/alicloud/actiontrail_trail_oss_bucket_is_publicly_accessible/query.rego @@ -14,12 +14,17 @@ CxPolicy[result] { result := { "documentId": input.document[i].id, - "resourceType": "alicloud_actiontrail_trail", - "resourceName": tf_lib.get_specific_resource_name(actiontrail, "alicloud_actiontrail_trail", name), - "searchKey": sprintf("alicloud_actiontrail_trail[%s].oss_bucket_name", [name]), + "resourceType": "alicloud_oss_bucket", + "resourceName": tf_lib.get_specific_resource_name(actiontrail, "alicloud_oss_bucket", name), + "searchKey": sprintf("alicloud_oss_bucket[%s].acl", [name]), "issueType": "IncorrectValue", - "keyExpectedValue": sprintf("'alicloud_actiontrail_trail[%s].oss_bucket_name' is private", [name]), - "keyActualValue": sprintf("'alicloud_actiontrail_trail[%s].oss_bucket_name' is %s", [name, possibilities[p]]), - "searchLine": common_lib.build_search_line(["resource", "alicloud_actiontrail_trail", name, "oss_bucket_name"], []), + "keyExpectedValue": sprintf("'alicloud_oss_bucket[%s].oss_bucket_name' is private", [name]), + "keyActualValue": sprintf("'alicloud_oss_bucket[%s].oss_bucket_name' is %s", [name, possibilities[p]]), + "searchLine": common_lib.build_search_line(["resource", "alicloud_oss_bucket", name, "acl"], []), + "remediation": json.marshal({ + "before": p, + "after": "private" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/actiontrail_trail_oss_bucket_is_publicly_accessible/test/positive_expected_result.json b/assets/queries/terraform/alicloud/actiontrail_trail_oss_bucket_is_publicly_accessible/test/positive_expected_result.json index cbd24b1d29f..2928f9afc52 100644 --- a/assets/queries/terraform/alicloud/actiontrail_trail_oss_bucket_is_publicly_accessible/test/positive_expected_result.json +++ b/assets/queries/terraform/alicloud/actiontrail_trail_oss_bucket_is_publicly_accessible/test/positive_expected_result.json @@ -2,13 +2,13 @@ { "queryName": "ActionTrail Trail OSS Bucket is Publicly Accessible", "severity": "HIGH", - "line": 9, + "line": 3, "fileName": "positive2.tf" }, { "queryName": "ActionTrail Trail OSS Bucket is Publicly Accessible", "severity": "HIGH", - "line": 9, + "line": 3, "fileName": "positive1.tf" } ] diff --git a/assets/queries/terraform/alicloud/alb_listening_on_http/query.rego b/assets/queries/terraform/alicloud/alb_listening_on_http/query.rego index 2f25d71edd3..e68cad6f295 100644 --- a/assets/queries/terraform/alicloud/alb_listening_on_http/query.rego +++ b/assets/queries/terraform/alicloud/alb_listening_on_http/query.rego @@ -16,5 +16,10 @@ CxPolicy[result] { "keyExpectedValue": "'alicloud_alb_listener[%s].listener_protocol' should not be 'HTTP'", "keyActualValue": "'alicloud_alb_listener[%s].listener_protocol' is 'HTTP'", "searchLine": common_lib.build_search_line(["resource", "alicloud_alb_listener", name, "listener_protocol"], []), + "remediation": json.marshal({ + "before": "HTTP", + "after": "HTTPS" + }), + "remediationType": "replacement" } } diff --git a/assets/queries/terraform/alicloud/api_gateway_api_protocol_not_https/query.rego b/assets/queries/terraform/alicloud/api_gateway_api_protocol_not_https/query.rego index cf3a70dcc44..c7f4a3693d5 100644 --- a/assets/queries/terraform/alicloud/api_gateway_api_protocol_not_https/query.rego +++ b/assets/queries/terraform/alicloud/api_gateway_api_protocol_not_https/query.rego @@ -17,6 +17,11 @@ CxPolicy[result] { "keyExpectedValue": "'protocol' value should be 'HTTPS'", "keyActualValue": "'protocol' value is 'HTTP' or 'HTTP,HTTPS'", "searchLine": common_lib.build_search_line(["resource", "alicloud_api_gateway_api", name, "request_config","protocol"], []), + "remediation": json.marshal({ + "before": request_config.protocol, + "after": "HTTPS" + }), + "remediationType": "replacement", } } @@ -34,5 +39,10 @@ CxPolicy[result] { "keyExpectedValue": "'protocol' value should be 'HTTPS'", "keyActualValue": "'protocol' value is 'HTTP' or 'HTTP,HTTPS'", "searchLine": common_lib.build_search_line(["resource", "alicloud_api_gateway_api", name, "request_config", index, "protocol" ], []), + "remediation": json.marshal({ + "before": request_config.protocol, + "after": "HTTPS" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/high_kms_key_rotation_period/query.rego b/assets/queries/terraform/alicloud/high_kms_key_rotation_period/query.rego index adb65e2df98..57b339d84c9 100644 --- a/assets/queries/terraform/alicloud/high_kms_key_rotation_period/query.rego +++ b/assets/queries/terraform/alicloud/high_kms_key_rotation_period/query.rego @@ -18,6 +18,11 @@ CxPolicy[result] { "keyExpectedValue": "'rotation_interval' value should not be higher than a year", "keyActualValue": "'rotation_interval' value is higher than a year", "searchLine": common_lib.build_search_line(["resource", "alicloud_kms_key", name, "rotation_interval"], []), + "remediation": json.marshal({ + "before": resource.rotation_interval, + "after": "365d" + }), + "remediationType": "replacement", } } @@ -35,6 +40,8 @@ CxPolicy[result] { "keyExpectedValue": "'automatic_rotation' should be defined and set to Enabled", "keyActualValue": "'automatic_rotation' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_kms_key", name], []), + "remediation": "automatic_rotation = \"Enabled\"", + "remediationType": "addition", } } @@ -52,6 +59,11 @@ CxPolicy[result] { "keyExpectedValue": "'automatic_rotation' should be set to Enabled", "keyActualValue": "'automatic_rotation' is set to Disabled", "searchLine": common_lib.build_search_line(["resource", "alicloud_kms_key", name, "automatic_rotation"], []), + "remediation": json.marshal({ + "before": "Disabled", + "after": "Enabled" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/log_retention_is_not_greater_than_90_days/query.rego b/assets/queries/terraform/alicloud/log_retention_is_not_greater_than_90_days/query.rego index 78637f40533..bbdc2893c01 100644 --- a/assets/queries/terraform/alicloud/log_retention_is_not_greater_than_90_days/query.rego +++ b/assets/queries/terraform/alicloud/log_retention_is_not_greater_than_90_days/query.rego @@ -17,6 +17,8 @@ CxPolicy[result] { "keyExpectedValue": "For attribute 'retention_period' to be set and over 90 days.", "keyActualValue": "The attribute 'retention_period' is undefined. The default duration when undefined is 30 days, which is too short.", "searchLine": common_lib.build_search_line(["resource", "alicloud_log_store", name], []), + "remediation": "retention_period = 100", + "remediationType": "addition", } } @@ -35,5 +37,10 @@ CxPolicy[result] { "keyExpectedValue": "For the attribite 'retention_period' to be set to 90+ days", "keyActualValue": "The attribute 'retention_period' is not set to 90+ days", "searchLine": common_lib.build_search_line(["resource", "alicloud_log_store", name, "retention_period"], []), + "remediation": json.marshal({ + "before": sprintf("%d", [rperiod]), + "after": "100" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/nas_file_system_not_encrypted/query.rego b/assets/queries/terraform/alicloud/nas_file_system_not_encrypted/query.rego index 77bfc7e4488..8dcc8f8339a 100644 --- a/assets/queries/terraform/alicloud/nas_file_system_not_encrypted/query.rego +++ b/assets/queries/terraform/alicloud/nas_file_system_not_encrypted/query.rego @@ -16,6 +16,11 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_nas_file_system[%s].encrypt_type' should not be 0", [name]), "keyActualValue": sprintf("alicloud_nas_file_system[%s].encrypt_type' is 0", [name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_nas_file_system", name, "encrypt_type"], []), + "remediation": json.marshal({ + "before": "0", + "after": "2" + }), + "remediationType": "replacement", } } @@ -32,5 +37,7 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_nas_file_system[%s].encrypt_type' should be defined and the value different from 0 ", [name]), "keyActualValue": sprintf("alicloud_nas_file_system[%s].encrypt_type' is undefined", [name]), "searchLine":common_lib.build_search_line(["resource", "alicloud_nas_file_system", name], []), + "remediation": "encrypt_type = \"2\"", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/nas_file_system_without_kms/query.rego b/assets/queries/terraform/alicloud/nas_file_system_without_kms/query.rego index 1cc32cd2bbf..c89aed01f50 100644 --- a/assets/queries/terraform/alicloud/nas_file_system_without_kms/query.rego +++ b/assets/queries/terraform/alicloud/nas_file_system_without_kms/query.rego @@ -16,6 +16,8 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_nas_file_system[%s].encrypt_type' should be defined and set to 2'", [name]), "keyActualValue": sprintf("alicloud_nas_file_system[%s].encrypt_type' is not defined", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_nas_file_system", name], []), + "remediation": "encrypt_type = \"2\"", + "remediationType": "addition", } } @@ -32,5 +34,10 @@ CxPolicy[result] { "keyExpectedValue": sprintf("alicloud_nas_file_system[%s].encrypt_type' should be set to 2'", [name]), "keyActualValue": sprintf("alicloud_nas_file_system[%s].encrypt_type' is not set to 2 ", [name]), "searchLine": common_lib.build_search_line(["resource", "alicloud_nas_file_system", name, "encrypt_type"], []), + "remediation": json.marshal({ + "before": resource.encrypt_type, + "after": "2" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/oss_bucket_public_access_enabled/query.rego b/assets/queries/terraform/alicloud/oss_bucket_public_access_enabled/query.rego index a960d9afb9c..5a0958c23aa 100644 --- a/assets/queries/terraform/alicloud/oss_bucket_public_access_enabled/query.rego +++ b/assets/queries/terraform/alicloud/oss_bucket_public_access_enabled/query.rego @@ -19,5 +19,10 @@ CxPolicy[result] { "keyExpectedValue": "'acl' is set to private or not set", "keyActualValue": sprintf("'acl' is %s", [possibilities[p]]), "searchLine":common_lib.build_search_line(["resource", "alicloud_oss_bucket", name, "acl"], []), + "remediation": json.marshal({ + "before": p, + "after": "private" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/oss_bucket_versioning_disabled/query.rego b/assets/queries/terraform/alicloud/oss_bucket_versioning_disabled/query.rego index 7c26c492f1b..c212f5d1bb8 100644 --- a/assets/queries/terraform/alicloud/oss_bucket_versioning_disabled/query.rego +++ b/assets/queries/terraform/alicloud/oss_bucket_versioning_disabled/query.rego @@ -18,6 +18,11 @@ CxPolicy[result] { "keyExpectedValue": "'versioning.status' is enabled", "keyActualValue": "'versioning.status' is suspended", "searchLine": common_lib.build_search_line(["resource", "alicloud_oss_bucket", name, "versioning", "status"], []), + "remediation": json.marshal({ + "before": "Suspended", + "after": "Enabled" + }), + "remediationType": "replacement", } } @@ -36,5 +41,7 @@ CxPolicy[result] { "keyExpectedValue": "'versioning.status' is defined and set to enabled", "keyActualValue": "'versioning' is missing", "searchLine": common_lib.build_search_line(["resource", "alicloud_oss_bucket", name], []), + "remediation": "versioning {\n\t\tstatus = \"Enabled\"\n\t}", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_max_login_attempts_unrecommended/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_max_login_attempts_unrecommended/query.rego index cf83c51d2b0..a0375d585a7 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_max_login_attempts_unrecommended/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_max_login_attempts_unrecommended/query.rego @@ -17,6 +17,10 @@ CxPolicy[result] { "keyExpectedValue": "'max_login_attempts' is set to 5 or less", "keyActualValue": "'max_login_attempts' is above than 5", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "max_login_attempts"], []), - + "remediation": json.marshal({ + "before": sprintf("%d", [resource.max_login_attempts]), + "after": "5" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego index 25db9fc697f..662a3b36721 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_max_password_age_unrecommended/query.rego @@ -36,6 +36,11 @@ CxPolicy[result] { "keyExpectedValue": "'max_password_age' should be higher than 0 and lower than 91", "keyActualValue": "'max_password_age' is higher than 90", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "max_password_age"], []), + "remediation": json.marshal({ + "before": sprintf("%d", [resource.max_password_age]), + "after": "12" + }), + "remediationType": "replacement", } } @@ -54,6 +59,10 @@ CxPolicy[result] { "keyExpectedValue": "'max_password_age' should be higher than 0 and lower than 91", "keyActualValue": "'max_password_age' is equal to 0", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "max_password_age"], []), - + "remediation": json.marshal({ + "before": sprintf("%d", [resource.max_password_age]), + "after": "12" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_without_reuse_prevention/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_without_reuse_prevention/query.rego index dbdaa8d7f72..8700c85f888 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_without_reuse_prevention/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_without_reuse_prevention/query.rego @@ -17,7 +17,8 @@ CxPolicy[result] { "keyExpectedValue": "'password_reuse_prevention' is defined and equal or lower than 24", "keyActualValue": "'password_reuse_prevention' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name], []), - + "remediation": "password_reuse_prevention = 24", + "remediationType": "addition", } } @@ -35,6 +36,10 @@ CxPolicy[result] { "keyExpectedValue": "'password_reuse_prevention' should be equal or less 24", "keyActualValue": "'password_reuse_prevention' is higher than 24", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name, "password_reuse_prevention"], []), - + "remediation": json.marshal({ + "before": sprintf("%d", [resource.password_reuse_prevention]), + "after": "24" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/rds_instance_retention_not_recommended/query.rego b/assets/queries/terraform/alicloud/rds_instance_retention_not_recommended/query.rego index 862f8b1bb8c..a29b514837f 100644 --- a/assets/queries/terraform/alicloud/rds_instance_retention_not_recommended/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_retention_not_recommended/query.rego @@ -16,6 +16,8 @@ CxPolicy[result] { "keyExpectedValue": "'sql_collector_status' should be defined and set to Enabled and 'sql_collector_config_value' should be defined and set to 180 or more", "keyActualValue": "'sql_collector_status' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "remediation": "sql_collector_status = \"Enabled\"", + "remediationType": "addition", } } @@ -32,6 +34,11 @@ CxPolicy[result] { "keyExpectedValue": "'sql_collector_status' should be defined and set to Enabled and 'sql_collector_config_value' should be defined and set to 180 or more", "keyActualValue": "'sql_collector_status' is set to 'Disabled'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name,"sql_collector_status" ], []), + "remediation": json.marshal({ + "before": "Disabled", + "after": "Enabled" + }), + "remediationType": "replacement", } } @@ -49,6 +56,8 @@ CxPolicy[result] { "keyExpectedValue": "'sql_collector_status' should be defined and set to Enabled and 'sql_collector_config_value' should be defined and set to 180 or more", "keyActualValue": "'sql_collector_config_value' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "remediation": "sql_collector_config_value = 180", + "remediationType": "addition", } } @@ -65,5 +74,10 @@ CxPolicy[result] { "keyExpectedValue": "'sql_collector_status' should be defined and set to Enabled and 'sql_collector_config_value' should be defined and set to 180 or more", "keyActualValue": "'sql_collector_config_value' is set to 30", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name,"sql_collector_config_value" ], []), + "remediation": json.marshal({ + "before": "30", + "after": "180" + }), + "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/rds_instance_ssl_action_disabled/query.rego b/assets/queries/terraform/alicloud/rds_instance_ssl_action_disabled/query.rego index 3d01dee3102..c6f94eb1c0c 100644 --- a/assets/queries/terraform/alicloud/rds_instance_ssl_action_disabled/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_ssl_action_disabled/query.rego @@ -18,6 +18,11 @@ CxPolicy[result] { "keyExpectedValue": "'ssl_action' value should be 'Open'", "keyActualValue": "'ssl_action' value is 'Close'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name, "ssl_action"], []), + "remediation": json.marshal({ + "before": "Close", + "after": "Open" + }), + "remediationType": "replacement", } } @@ -35,5 +40,7 @@ CxPolicy[result] { "keyExpectedValue": "'ssl_action' value should be 'Open'", "keyActualValue": "'ssl_action' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "remediation": "ssl_action = \"Open\"", + "remediationType": "addition", } } diff --git a/assets/queries/terraform/alicloud/rds_instance_tde_status_disable/query.rego b/assets/queries/terraform/alicloud/rds_instance_tde_status_disable/query.rego index 13a91157ecc..f41d9f6ab82 100644 --- a/assets/queries/terraform/alicloud/rds_instance_tde_status_disable/query.rego +++ b/assets/queries/terraform/alicloud/rds_instance_tde_status_disable/query.rego @@ -22,6 +22,11 @@ CxPolicy[result] { "keyExpectedValue": "'tde_status' value should be 'Enabled'", "keyActualValue": "'tde_status' value is set to 'Disabled'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name, "tde_status"], []), + "remediation": json.marshal({ + "before": "Disabled", + "after": "Enabled" + }), + "remediationType": "replacement", } } @@ -41,6 +46,8 @@ CxPolicy[result] { "keyExpectedValue": "'tde_status' value should be 'Enabled'", "keyActualValue": "'tde_status' is not declared", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "remediation": "tde_status = \"Enabled\"", + "remediationType": "addition", } } @@ -60,6 +67,11 @@ CxPolicy[result] { "keyExpectedValue": "'tde_status' value should be 'Enabled'", "keyActualValue": "'tde_status' value is set to 'Disabled'", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name, "tde_status"], []), + "remediation": json.marshal({ + "before": "Disabled", + "after": "Enabled" + }), + "remediationType": "replacement", } } @@ -79,6 +91,8 @@ CxPolicy[result] { "keyExpectedValue": "'tde_status' value should be 'Enabled'", "keyActualValue": "'tde_status' is not declared", "searchLine": common_lib.build_search_line(["resource", "alicloud_db_instance", name], []), + "remediation": "tde_status = \"Enabled\"", + "remediationType": "addition", } } diff --git a/res/positive1.tf b/res/positive1.tf deleted file mode 100644 index a48eed94674..00000000000 --- a/res/positive1.tf +++ /dev/null @@ -1,4 +0,0 @@ -resource "alicloud_oss_bucket" "bucket_logging2" { - bucket = "bucket-170309-acl" - acl = "public-read" -} diff --git a/res/positive2.tf b/res/positive2.tf deleted file mode 100644 index b7a98bb3564..00000000000 --- a/res/positive2.tf +++ /dev/null @@ -1,9 +0,0 @@ -resource "alicloud_oss_bucket" "bucket_logging1" { - bucket = "bucket-170309-logging" - logging_isenable = true - - logging { - target_bucket = alicloud_oss_bucket.bucket-target.id - target_prefix = "log/" - } -} From 6f6c27eab92758ee17ee7c1756bec7ee052e0cc3 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Fri, 8 Jul 2022 10:50:19 +0100 Subject: [PATCH 07/22] added E2E tests --- .../query.rego | 2 +- docs/commands.md | 1 + docs/kics-auto-remediation.md | 30 +++ e2e/cli_test.go | 10 + e2e/fixtures/E2E_CLI_059 | 3 + e2e/fixtures/E2E_CLI_060 | 2 + e2e/fixtures/assets/fix_help | 18 ++ e2e/fixtures/assets/help | 1 + .../kics-auto-remediation/terraform.tf | 20 ++ e2e/testcases/e2e-cli-003_scan_text.go | 5 +- e2e/testcases/e2e-cli-057_fix_all.go | 47 +++++ e2e/testcases/e2e-cli-058_fix_include_ids.go | 50 +++++ e2e/testcases/e2e-cli-059_help_fix.go | 18 ++ e2e/testcases/e2e-cli-060_fix_text.go | 18 ++ e2e/testcases/general.go | 1 + e2e/testcases/utils.go | 175 ++++++++++++++++++ internal/console/assets/fix-flags.json | 4 +- internal/console/fix.go | 15 +- internal/console/helpers/exit_handler.go | 11 ++ pkg/remediation/remediation.go | 9 +- pkg/remediation/remediation_test.go | 45 ++--- pkg/remediation/scan.go | 36 +++- pkg/remediation/utils.go | 37 +++- pkg/remediation/utils_test.go | 54 ++++-- 24 files changed, 541 insertions(+), 71 deletions(-) create mode 100644 docs/kics-auto-remediation.md create mode 100644 e2e/fixtures/E2E_CLI_059 create mode 100644 e2e/fixtures/E2E_CLI_060 create mode 100644 e2e/fixtures/assets/fix_help create mode 100644 e2e/fixtures/samples/kics-auto-remediation/terraform.tf create mode 100644 e2e/testcases/e2e-cli-057_fix_all.go create mode 100644 e2e/testcases/e2e-cli-058_fix_include_ids.go create mode 100644 e2e/testcases/e2e-cli-059_help_fix.go create mode 100644 e2e/testcases/e2e-cli-060_fix_text.go create mode 100644 e2e/testcases/utils.go diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego index 27500ed5d93..7c54ebd799c 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego @@ -37,7 +37,7 @@ CxPolicy[result] { "searchKey": sprintf("alicloud_ram_account_password_policy[%s]", [name]), "issueType": "MissingAttribute", "keyExpectedValue": "'minimum_password_length' is defined and set to 14 or above ", - "keyActualValue": "'minimum_password_length' is not difined", + "keyActualValue": "'minimum_password_length' is not defined", "searchLine": common_lib.build_search_line(["resource", "alicloud_ram_account_password_policy", name], []), "remediation": "minimum_password_length = 14", "remediationType": "addition", diff --git a/docs/commands.md b/docs/commands.md index 5c0a65ba46c..e949c0143dc 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -13,6 +13,7 @@ Usage: kics [command] Available Commands: + fix Auto remediates the project generate-id Generates uuid for query help Help about any command list-platforms List supported platforms diff --git a/docs/kics-auto-remediation.md b/docs/kics-auto-remediation.md new file mode 100644 index 00000000000..7c85f52dc92 --- /dev/null +++ b/docs/kics-auto-remediation.md @@ -0,0 +1,30 @@ +# KICS AUTO REMEDIATION + +With this new feature, KICS provides auto remediation for simple replacements and simple additions in a single line. + +Note that this feature will be only available for Terraform, for now. + +

+image +

+ + + +## HOW KICS AR WORKS? + + +1. As a first step, you will need to scan your project/file and generate a JSON report. + + Example: ```docker run -v /home/cosmicgirl/:/path/ kics scan -p /path/sample.tf -i "41a38329-d81b-4be4-aef4-55b2615d3282,a9dfec39-a740-4105-bbd6-721ba163c053,2bb13841-7575-439e-8e0a-cccd9ede2fa8" --no-progress -o /path/results --report-formats json``` + + If KICS makes available a remediation for a result, the result will have the fields `remediation` and `remediation_type` defined. As an example, please see: +

+ image +

+ + +2. If your JSON report has any result with remediation, you will need to run the new KICS command: fix. + + If you want KICS to remediate all the remediation, you can run ```docker run -v /home/cosmicgirl/:/path/ kics fix --results /path/results/results.json -v```. + + If you want to specify which remediation KICS should fix, you can use the flag `--include-ids`. In this flag, you should point the `similarity_id` of the result. For example: ```docker run -v /home/cosmicgirl/:/path/ kics fix --results /path/results/results.json --include-ids "f282fa13cf5e4ffd4bbb0ee2059f8d0240edcd2ca54b3bb71633145d961de5ce" -v``` diff --git a/e2e/cli_test.go b/e2e/cli_test.go index 31d94eb5c7e..a44b203be7b 100644 --- a/e2e/cli_test.go +++ b/e2e/cli_test.go @@ -119,6 +119,10 @@ func Test_E2E_CLI(t *testing.T) { if err != nil { t.Logf("\nError when trying to remove tests output folder\n") } + err = os.RemoveAll("fixtures/tmp-kics-ar") + if err != nil { + t.Logf("\nError when trying to remove fixtures tmp-kics-ar folder\n") + } t.Logf("E2E tests ::ellapsed time:: %v", time.Since(scanStartTime)) }) } @@ -202,9 +206,15 @@ func prepareTemplates() testcases.TestTemplates { scanHelp = []string{} } + var fixHelp, errFH = utils.PrepareExpected("fix_help", "fixtures/assets") + if errFH != nil { + fixHelp = []string{} + } + return testcases.TestTemplates{ Help: strings.Join(help, "\n"), ScanHelp: strings.Join(scanHelp, "\n"), + FixHelp: strings.Join(fixHelp, "\n"), } } diff --git a/e2e/fixtures/E2E_CLI_059 b/e2e/fixtures/E2E_CLI_059 new file mode 100644 index 00000000000..eef12273a7b --- /dev/null +++ b/e2e/fixtures/E2E_CLI_059 @@ -0,0 +1,3 @@ +Auto remediates the project + +{{.FixHelp}} \ No newline at end of file diff --git a/e2e/fixtures/E2E_CLI_060 b/e2e/fixtures/E2E_CLI_060 new file mode 100644 index 00000000000..d24adf83b92 --- /dev/null +++ b/e2e/fixtures/E2E_CLI_060 @@ -0,0 +1,2 @@ +Error: required flag(s) "results" not set +{{.FixHelp}} diff --git a/e2e/fixtures/assets/fix_help b/e2e/fixtures/assets/fix_help new file mode 100644 index 00000000000..c6da4a60582 --- /dev/null +++ b/e2e/fixtures/assets/fix_help @@ -0,0 +1,18 @@ +Usage: + kics fix [flags] + +Flags: + -h, --help help for fix + --include-ids strings which remediation (similarity ids) should be remediated + example "f6b7acac2d541d8c15c88d2be51b0e6abd576750b71c580f2e3a9346f7ed0e67,6af5fc5d7c0ad0077348a090f7c09949369d24d5608bbdbd14376a15de62afd1" (default [all]) + --results string points to the JSON results file with remediation + +Global Flags: + --ci display only log messages to CLI output (mutually exclusive with silent) + -f, --log-format string determines log format (pretty,json) (default "pretty") + --log-level string determines log level (TRACE,DEBUG,INFO,WARN,ERROR,FATAL) (default "INFO") + --log-path string path to generate log file (info.log) + --no-color disable CLI color output + --profiling string enables performance profiler that prints resource consumption metrics in the logs during the execution (CPU, MEM) + -s, --silent silence stdout messages (mutually exclusive with verbose and ci) + -v, --verbose write logs to stdout too (mutually exclusive with silent) diff --git a/e2e/fixtures/assets/help b/e2e/fixtures/assets/help index 7519be10da4..650f712aa38 100644 --- a/e2e/fixtures/assets/help +++ b/e2e/fixtures/assets/help @@ -2,6 +2,7 @@ Usage: kics [command] Available Commands: + fix Auto remediates the project generate-id Generates uuid for query help Help about any command list-platforms List supported platforms diff --git a/e2e/fixtures/samples/kics-auto-remediation/terraform.tf b/e2e/fixtures/samples/kics-auto-remediation/terraform.tf new file mode 100644 index 00000000000..ff7e89a967e --- /dev/null +++ b/e2e/fixtures/samples/kics-auto-remediation/terraform.tf @@ -0,0 +1,20 @@ +resource "alicloud_ram_account_password_policy" "corporate1" { + require_lowercase_characters = false + require_uppercase_characters = false + require_numbers = false + require_symbols = false + hard_expiry = true + password_reuse_prevention = 5 + max_login_attempts = 3 +} + +resource "alicloud_ram_account_password_policy" "corporate2" { + minimum_password_length = 14 + require_lowercase_characters = false + require_uppercase_characters = false + require_numbers = false + require_symbols = false + hard_expiry = true + password_reuse_prevention = 5 + max_login_attempts = 3 +} diff --git a/e2e/testcases/e2e-cli-003_scan_text.go b/e2e/testcases/e2e-cli-003_scan_text.go index 48b516a22a5..d1741de4468 100644 --- a/e2e/testcases/e2e-cli-003_scan_text.go +++ b/e2e/testcases/e2e-cli-003_scan_text.go @@ -1,8 +1,7 @@ package testcases -// E2E-CLI-003 - KICS scan command had a mandatory flag -p the CLI should exhibit -// an error message and return exit code 1 - +// E2E-CLI-003 - KICS scan command has a mandatory flag -p. The CLI should exhibit +// an error message and return exit code 126 func init() { //nolint testSample := TestCase{ Name: "should display an error regarding missing -p flag [E2E-CLI-003]", diff --git a/e2e/testcases/e2e-cli-057_fix_all.go b/e2e/testcases/e2e-cli-057_fix_all.go new file mode 100644 index 00000000000..ee7b0a91cdd --- /dev/null +++ b/e2e/testcases/e2e-cli-057_fix_all.go @@ -0,0 +1,47 @@ +package testcases + +import ( + "os" + "path/filepath" + "regexp" + + "github.com/Checkmarx/kics/pkg/remediation" + "github.com/Checkmarx/kics/pkg/utils" + "github.com/rs/zerolog/log" +) + +// E2E-CLI-057 - Kics fix command +// should fix all remediation found +func init() { // nolint + if err := os.MkdirAll("./fixtures/tmp-kics-ar", os.ModePerm); err != nil { + log.Error().Msgf("failed to mkdir: %s", err) + } + + filePath := "./fixtures/samples/kics-auto-remediation/terraform.tf" + tmpFilePath := "./fixtures/tmp-kics-ar/temporary-remediation" + utils.NextRandom() + filepath.Ext(filePath) + jsonPath := "./fixtures/tmp-kics-ar" + + // create a temporary file with the same content as filePath + tmpFile := remediation.CreateTempFile(filePath, tmpFilePath) + + // create JSON results with remediation + generateReport(tmpFile, jsonPath) + pathResults := filepath.Join(jsonPath, "results.json") + + testSample := TestCase{ + Name: "should fix all remediation found [E2E-CLI-057]", + Args: args{ + Args: []cmdArgs{ + []string{"fix", "--results", pathResults, "-v"}, + }, + }, + WantStatus: []int{0}, + Validation: func(outputText string) bool { + match1, _ := regexp.MatchString(`Selected remediation: 5`, outputText) + match2, _ := regexp.MatchString(`Remediation done: 5`, outputText) + return match1 && match2 + }, + } + + Tests = append(Tests, testSample) +} diff --git a/e2e/testcases/e2e-cli-058_fix_include_ids.go b/e2e/testcases/e2e-cli-058_fix_include_ids.go new file mode 100644 index 00000000000..b0b727b77c6 --- /dev/null +++ b/e2e/testcases/e2e-cli-058_fix_include_ids.go @@ -0,0 +1,50 @@ +package testcases + +import ( + "os" + "path/filepath" + "regexp" + + "github.com/Checkmarx/kics/pkg/remediation" + "github.com/Checkmarx/kics/pkg/utils" + "github.com/rs/zerolog/log" +) + +// E2E-CLI-058 - Kics fix command with include-ids flag +// should fix the recomendations pointed in include-ids flag +func init() { // nolint + if err := os.MkdirAll("./fixtures/tmp-kics-ar", os.ModePerm); err != nil { + log.Error().Msgf("failed to mkdir: %s", err) + } + + filePath := "./fixtures/samples/kics-auto-remediation/terraform.tf" + tmpFilePath := "./fixtures/tmp-kics-ar/temporary-remediation" + utils.NextRandom() + filepath.Ext(filePath) + jsonPath := "./fixtures/tmp-kics-ar" + + // create a temporary file with the same content as filePath + tmpFile := remediation.CreateTempFile(filePath, tmpFilePath) + + // create JSON results with remediation + generateReport(tmpFile, jsonPath) + pathResults := filepath.Join(jsonPath, "results.json") + + testSample := TestCase{ + Name: "should fix the recomendations pointed in include-ids flag [E2E-CLI-058]", + Args: args{ + Args: []cmdArgs{ + []string{"fix", "--results", pathResults, + "--include-ids", "f282fa13cf5e4ffd4bbb0ee2059f8d0240edcd2ca54b3bb71633145d961de5ce," + + "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", + "-v"}, + }, + }, + WantStatus: []int{0}, + Validation: func(outputText string) bool { + match1, _ := regexp.MatchString(`Selected remediation: 2`, outputText) + match2, _ := regexp.MatchString(`Remediation done: 2`, outputText) + return match1 && match2 + }, + } + + Tests = append(Tests, testSample) +} diff --git a/e2e/testcases/e2e-cli-059_help_fix.go b/e2e/testcases/e2e-cli-059_help_fix.go new file mode 100644 index 00000000000..26c061ab9f8 --- /dev/null +++ b/e2e/testcases/e2e-cli-059_help_fix.go @@ -0,0 +1,18 @@ +package testcases + +// E2E-CLI-059 - KICS fix command should display a help text in the CLI when provided with the +// --help flag and it should describe the options related with fix plus the global options +func init() { //nolint + testSample := TestCase{ + Name: "should display the kics fix help text [E2E-CLI-059]", + Args: args{ + Args: []cmdArgs{ + []string{"fix", "--help"}, + }, + ExpectedOut: []string{"E2E_CLI_059"}, + }, + WantStatus: []int{0}, + } + + Tests = append(Tests, testSample) +} diff --git a/e2e/testcases/e2e-cli-060_fix_text.go b/e2e/testcases/e2e-cli-060_fix_text.go new file mode 100644 index 00000000000..9b0f3797c54 --- /dev/null +++ b/e2e/testcases/e2e-cli-060_fix_text.go @@ -0,0 +1,18 @@ +package testcases + +// E2E-CLI-060 - KICS fix command has a mandatory flag --results. The CLI should exhibit +// an error message and return exit code 126 +func init() { //nolint + testSample := TestCase{ + Name: "should display an error regarding missing --results flag [E2E-CLI-060]", + Args: args{ + Args: []cmdArgs{ + []string{"fix"}, + }, + ExpectedOut: []string{"E2E_CLI_060"}, + }, + WantStatus: []int{126}, + } + + Tests = append(Tests, testSample) +} diff --git a/e2e/testcases/general.go b/e2e/testcases/general.go index fe79c344018..f4e37b8f29e 100644 --- a/e2e/testcases/general.go +++ b/e2e/testcases/general.go @@ -31,6 +31,7 @@ type args struct { type TestTemplates struct { Help string ScanHelp string + FixHelp string } type cmdArgs []string diff --git a/e2e/testcases/utils.go b/e2e/testcases/utils.go new file mode 100644 index 00000000000..f35aa05feff --- /dev/null +++ b/e2e/testcases/utils.go @@ -0,0 +1,175 @@ +package testcases + +import ( + "github.com/Checkmarx/kics/pkg/model" + "github.com/Checkmarx/kics/pkg/report" + "github.com/rs/zerolog/log" +) + +func generateReport(tmpFile, jsonPath string) { // nolint + var queryHigh = model.QueryResult{ + QueryName: "Ram Account Password Policy Not Required Minimum Length", + QueryID: "a9dfec39-a740-4105-bbd6-721ba163c053", + QueryURI: "", + Description: "Ram Account Password Policy should have 'minimum_password_length' defined and set to 14 or above", + DescriptionID: "a8b47743", + CISDescriptionIDFormatted: "testCISID", + CISDescriptionTitle: "testCISTitle", + CISDescriptionTextFormatted: "testCISDescription", + Severity: model.SeverityHigh, + Platform: "Terraform", + CloudProvider: "ALICLOUD", + Category: "Secret Management", + Files: []model.VulnerableFile{ + { + FileName: tmpFile, + SimilarityID: "f282fa13cf5e4ffd4bbb0ee2059f8d0240edcd2ca54b3bb71633145d961de5ce", + Line: 1, + ResourceType: "alicloud_ram_account_password_policy", + ResourceName: "corporate1", + IssueType: "MissingAttribute", + SearchKey: "alicloud_ram_account_password_policy[corporate1]", + SearchLine: 0, + SearchValue: "", + KeyExpectedValue: "'minimum_password_length' is defined and set to 14 or above ", + KeyActualValue: "'minimum_password_length' is not defined", + Value: nil, + Remediation: "minimum_password_length = 14", + RemediationType: "addition", + }, + }, + } + + var queryMedium1 = model.QueryResult{ // nolint + QueryName: "RAM Account Password Policy Not Required Symbols", + QueryID: "41a38329-d81b-4be4-aef4-55b2615d3282", + QueryURI: "", + Description: "RAM account password security should require at least one symbol", + DescriptionID: "f3616c34", + CISDescriptionIDFormatted: "testCISID", + CISDescriptionTitle: "testCISTitle", + CISDescriptionTextFormatted: "testCISDescription", + Severity: model.SeverityMedium, + Platform: "Terraform", + CloudProvider: "ALICLOUD", + Category: "Secret Management", + Files: []model.VulnerableFile{ + { + FileName: tmpFile, + SimilarityID: "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", + Line: 5, + ResourceType: "alicloud_ram_account_password_policy", + ResourceName: "corporate1", + IssueType: "IncorrectValue", + SearchKey: "resource.alicloud_ram_account_password_policy[corporate1].require_symbols", + SearchLine: 0, + SearchValue: "", + KeyExpectedValue: "resource.alicloud_ram_account_password_policy[corporate1].require_symbols is set to 'true'", + KeyActualValue: "resource.alicloud_ram_account_password_policy[corporate1].require_symbols is configured as 'false'", + Value: nil, + Remediation: "{\"after\":\"true\",\"before\":\"false\"}", + RemediationType: "replacement", + }, + { + FileName: tmpFile, + SimilarityID: "2628457bdb548986936dbd7d8479524f2079f26d36b9faa9f34423e796fe62c8", + Line: 16, + ResourceType: "alicloud_ram_account_password_policy", + ResourceName: "corporate2", + IssueType: "IncorrectValue", + SearchKey: "resource.alicloud_ram_account_password_policy[corporate2].require_symbols", + SearchLine: 0, + SearchValue: "", + KeyExpectedValue: "resource.alicloud_ram_account_password_policy[corporate2].require_symbols is set to 'true'", + KeyActualValue: "resource.alicloud_ram_account_password_policy[corporate2].require_symbols is configured as 'false'", + Value: nil, + Remediation: "{\"after\":\"true\",\"before\":\"false\"}", + RemediationType: "replacement", + }, + }, + } + + var queryMedium2 = model.QueryResult{ // nolint + QueryName: "Ram Account Password Policy Max Password Age Unrecommended", + QueryID: "2bb13841-7575-439e-8e0a-cccd9ede2fa8", + Description: "Ram Account Password Policy Password 'max_password_age' should be higher than 0 and lower than 91", + QueryURI: "", + DescriptionID: "6056f5ca", + CISDescriptionIDFormatted: "testCISID", + CISDescriptionTitle: "testCISTitle", + CISDescriptionTextFormatted: "testCISDescription", + Severity: model.SeverityMedium, + Platform: "Terraform", + CloudProvider: "ALICLOUD", + Category: "Secret Management", + Files: []model.VulnerableFile{ + { + FileName: tmpFile, + SimilarityID: "f1d17b3513439e03cd0a25690acc44755d4e68decfaa6c03522b20a65b26b617", + Line: 5, + ResourceType: "alicloud_ram_account_password_policy", + ResourceName: "corporate1", + IssueType: "MissingAttribute", + SearchKey: "alicloud_ram_account_password_policy[corporate1]", + SearchLine: 0, + SearchValue: "", + KeyExpectedValue: "'max_password_age' should be higher than 0 and lower than 91", + KeyActualValue: "'max_password_age' is not defined", + Value: nil, + Remediation: "max_password_age = 12", + RemediationType: "addition", + }, + { + FileName: tmpFile, + SimilarityID: "404ad93f4a485d0dd1b1621489c38be9c98dcc0b94396701ecad162e28db97fd", + Line: 11, + ResourceType: "alicloud_ram_account_password_policy", + ResourceName: "corporate2", + IssueType: "MissingAttribute", + SearchKey: "alicloud_ram_account_password_policy[corporate2]", + SearchLine: 0, + SearchValue: "", + KeyExpectedValue: "'max_password_age' should be higher than 0 and lower than 91", + KeyActualValue: "'max_password_age' is not defined", + Value: nil, + Remediation: "max_password_age = 12", + RemediationType: "addition", + }, + }, + } + + var summary = model.Summary{ + Counters: model.Counters{ + ScannedFiles: 1, + ParsedFiles: 1, + FailedToScanFiles: 0, + TotalQueries: 3, + FailedToExecuteQueries: 0, + }, + Queries: []model.QueryResult{ + queryHigh, + queryMedium1, + queryMedium2, + }, + SeveritySummary: model.SeveritySummary{ + ScanID: "console", + SeverityCounters: map[model.Severity]int{ + model.SeverityInfo: 0, + model.SeverityLow: 0, + model.SeverityMedium: 4, + model.SeverityHigh: 1, + }, + TotalCounter: 5, + }, + ScannedPaths: []string{ + tmpFile, + }, + } + + // create JSON report + err := report.PrintJSONReport(jsonPath, "results", summary) + + if err != nil { + log.Error().Msgf("failed to create JSON report: %s", err) + } +} diff --git a/internal/console/assets/fix-flags.json b/internal/console/assets/fix-flags.json index 29f276bcccc..5c21e5b726f 100644 --- a/internal/console/assets/fix-flags.json +++ b/internal/console/assets/fix-flags.json @@ -3,13 +3,13 @@ "flagType": "multiStr", "shorthandFlag": "", "defaultValue": "all", - "usage": "which remedations (similarity ids) should be remediated \nexample \"f6b7acac2d541d8c15c88d2be51b0e6abd576750b71c580f2e3a9346f7ed0e67,6af5fc5d7c0ad0077348a090f7c09949369d24d5608bbdbd14376a15de62afd1\"" + "usage": "which remediation (similarity ids) should be remediated \nexample \"f6b7acac2d541d8c15c88d2be51b0e6abd576750b71c580f2e3a9346f7ed0e67,6af5fc5d7c0ad0077348a090f7c09949369d24d5608bbdbd14376a15de62afd1\"" }, "results": { "flagType": "str", "shorthandFlag": "", "defaultValue": "", - "usage": "points to the JSON results file with remediations" + "usage": "points to the JSON results file with remediation" } } \ No newline at end of file diff --git a/internal/console/fix.go b/internal/console/fix.go index c29efbee057..64278f77009 100644 --- a/internal/console/fix.go +++ b/internal/console/fix.go @@ -7,6 +7,7 @@ import ( "os" "github.com/Checkmarx/kics/internal/console/flags" + consoleHelpers "github.com/Checkmarx/kics/internal/console/helpers" sentryReport "github.com/Checkmarx/kics/internal/sentry" "github.com/Checkmarx/kics/pkg/engine/source" internalPrinter "github.com/Checkmarx/kics/pkg/printer" @@ -92,10 +93,11 @@ func fix(cmd *cobra.Command) error { } summary := &remediation.Summary{ - SelectedRemediationsNumber: 0, - ActualRemediationsDoneNumber: 0, + SelectedRemediationNumber: 0, + ActualRemediationDoneNumber: 0, } + // get all the fixs related to each filePath fixs := summary.GetFixs(results, include) for filePath := range fixs { @@ -106,8 +108,13 @@ func fix(cmd *cobra.Command) error { } } - fmt.Printf("\nSelected remediations: %d\n", summary.SelectedRemediationsNumber) - fmt.Printf("Remediations done: %d\n", summary.ActualRemediationsDoneNumber) + fmt.Printf("\nSelected remediation: %d\n", summary.SelectedRemediationNumber) + fmt.Printf("Remediation done: %d\n", summary.ActualRemediationDoneNumber) + + exitCode := consoleHelpers.FixExitCode(summary.SelectedRemediationNumber, summary.ActualRemediationDoneNumber) + if exitCode != 0 { + os.Exit(exitCode) + } return nil } diff --git a/internal/console/helpers/exit_handler.go b/internal/console/helpers/exit_handler.go index 47df12dca2b..b7f9301b7b3 100644 --- a/internal/console/helpers/exit_handler.go +++ b/internal/console/helpers/exit_handler.go @@ -69,3 +69,14 @@ func InitShouldFailArg(args []string) error { func ShowError(kind string) bool { return strings.EqualFold(shouldIgnore, "none") || (!strings.EqualFold(shouldIgnore, "all") && !strings.EqualFold(shouldIgnore, kind)) } + +// FixExitCode calculate exit code base on the difference between remediation selected and done +func FixExitCode(selectedRemediationNumber, actualRemediationDoneNumber int) int { + statusCode := 70 + if selectedRemediationNumber != actualRemediationDoneNumber { + // KICS AR was not able to remediate all the selected remediation + return statusCode + } + + return 0 +} diff --git a/pkg/remediation/remediation.go b/pkg/remediation/remediation.go index 5eff1001dc3..b6802ecf303 100644 --- a/pkg/remediation/remediation.go +++ b/pkg/remediation/remediation.go @@ -55,7 +55,7 @@ func (s *Summary) RemediateFile(filePath string, fix Fix) error { r := fix.Replacement[i] remediatedLines := replacement(r, lines) if len(remediatedLines) > 0 && willRemediate(remediatedLines, filePath, &r) { - lines = s.writeRemediations(remediatedLines, lines, filePath, r.SimilarityID) + lines = s.writeRemediation(remediatedLines, lines, filePath, r.SimilarityID) } } } @@ -71,7 +71,7 @@ func (s *Summary) RemediateFile(filePath string, fix Fix) error { a := fix.Addition[i] remediatedLines := addition(a, &lines) if len(remediatedLines) > 0 && willRemediate(remediatedLines, filePath, &a) { - lines = s.writeRemediations(remediatedLines, lines, filePath, a.SimilarityID) + lines = s.writeRemediation(remediatedLines, lines, filePath, a.SimilarityID) } } } @@ -118,6 +118,7 @@ func addition(r Remediation, lines *[]string) []string { log.Info().Msgf("remediation '%s' is already done", r.SimilarityID) return []string{} } + begin := make([]string, len(*lines)) end := make([]string, len(*lines)) @@ -136,7 +137,7 @@ func addition(r Remediation, lines *[]string) []string { return remediation } -func (s *Summary) writeRemediations(remediatedLines, lines []string, filePath, similarityID string) []string { +func (s *Summary) writeRemediation(remediatedLines, lines []string, filePath, similarityID string) []string { remediated := []byte(strings.Join(remediatedLines, "\n")) if err := os.WriteFile(filePath, remediated, os.ModePerm); err != nil { @@ -145,7 +146,7 @@ func (s *Summary) writeRemediations(remediatedLines, lines []string, filePath, s } log.Info().Msgf("file '%s' was remediated with '%s'", filePath, similarityID) - s.ActualRemediationsDoneNumber++ + s.ActualRemediationDoneNumber++ return remediatedLines } diff --git a/pkg/remediation/remediation_test.go b/pkg/remediation/remediation_test.go index 50c47b080a3..bd77b0f6b35 100644 --- a/pkg/remediation/remediation_test.go +++ b/pkg/remediation/remediation_test.go @@ -50,37 +50,37 @@ func Test_RemediateFile(t *testing.T) { fix3.Replacement = append(fix3.Replacement, *wrongReplacement) tests := []struct { - name string - args args - actualRemediationsDoneNumber int + name string + args args + actualRemediationDoneNumber int }{ { name: "remediate a file with a replacement", args: args{ fix: fix1, }, - actualRemediationsDoneNumber: 1, + actualRemediationDoneNumber: 1, }, { name: "remediate a file with a replacement and addition", args: args{ fix: fix2, }, - actualRemediationsDoneNumber: 2, + actualRemediationDoneNumber: 2, }, { name: "remediate a file without any remediation", args: args{ fix: Fix{}, }, - actualRemediationsDoneNumber: 0, + actualRemediationDoneNumber: 0, }, { name: "remediate a file with a wrong remediation", args: args{ fix: fix3, }, - actualRemediationsDoneNumber: 0, + actualRemediationDoneNumber: 0, }, } @@ -107,38 +107,17 @@ func Test_RemediateFile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := &Summary{ - SelectedRemediationsNumber: 0, - ActualRemediationsDoneNumber: 0, + SelectedRemediationNumber: 0, + ActualRemediationDoneNumber: 0, } - tmpFile := createTempFile(filePath) + tmpFileName := filepath.Join(os.TempDir(), "temporary-remediation"+utils.NextRandom()+filepath.Ext(filePath)) + tmpFile := CreateTempFile(filePath, tmpFileName) s.RemediateFile(tmpFile, tt.args.fix) os.Remove(tmpFile) - require.Equal(t, s.ActualRemediationsDoneNumber, tt.actualRemediationsDoneNumber) + require.Equal(t, s.ActualRemediationDoneNumber, tt.actualRemediationDoneNumber) }) } } - -func createTempFile(filePath string) string { - // create temporary file - tmpFile := filepath.Join(os.TempDir(), "temporary-remediation"+utils.NextRandom()+filepath.Ext(filePath)) - f, err := os.OpenFile(tmpFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm) - - if err != nil { - f.Close() - return "" - } - - content, err := os.ReadFile(filePath) - if err != nil { - return "" - } - - if _, err = f.Write(content); err != nil { - f.Close() - return "" - } - return tmpFile -} diff --git a/pkg/remediation/scan.go b/pkg/remediation/scan.go index b6c345b054e..00117ef9fc4 100644 --- a/pkg/remediation/scan.go +++ b/pkg/remediation/scan.go @@ -10,6 +10,7 @@ import ( "github.com/Checkmarx/kics/pkg/engine" "github.com/Checkmarx/kics/pkg/kics" "github.com/Checkmarx/kics/pkg/model" + "github.com/Checkmarx/kics/pkg/scan" "github.com/open-policy-agent/opa/topdown" "github.com/Checkmarx/kics/internal/console/flags" @@ -35,6 +36,7 @@ type runQueryInfo struct { files model.FileMetadatas } +// scanTmpFile scans a temporary file against a specific query func scanTmpFile(tmpFile, queryID string, remediated []byte) ([]model.Vulnerability, error) { // get payload files, err := getPayload(tmpFile, remediated) @@ -76,6 +78,7 @@ func scanTmpFile(tmpFile, queryID string, remediated []byte) ([]model.Vulnerabil return runQuery(info), nil } +// getPayload gets the payload of a file func getPayload(filePath string, content []byte) (model.FileMetadatas, error) { ext := utils.GetExtension(filePath) var p []*parser.Parser @@ -140,6 +143,7 @@ func getPayload(filePath string, content []byte) (model.FileMetadatas, error) { return files, nil } +// runQuery runs a query and returns its results func runQuery(r *runQueryInfo) []model.Vulnerability { queryExecTimeout := time.Duration(flags.GetIntFlag(flags.QueryExecTimeoutFlag)) * time.Second @@ -177,11 +181,31 @@ func runQuery(r *runQueryInfo) []model.Vulnerability { } func initScan(queryID string) (*engine.Inspector, error) { + scanParams := &scan.Parameters{ + QueriesPath: flags.GetMultiStrFlag(flags.QueriesPath), + Platform: flags.GetMultiStrFlag(flags.TypeFlag), + CloudProvider: flags.GetMultiStrFlag(flags.CloudProviderFlag), + LibrariesPath: flags.GetStrFlag(flags.LibrariesPath), + PreviewLines: flags.GetIntFlag(flags.PreviewLinesFlag), + QueryExecTimeout: flags.GetIntFlag(flags.QueryExecTimeoutFlag), + } + + c := &scan.Client{ + ScanParams: scanParams, + } + + err := c.GetQueryPath() + + if err != nil { + log.Err(err) + return &engine.Inspector{}, err + } + queriesSource := source.NewFilesystemSource( - flags.GetMultiStrFlag(flags.QueriesPath), - flags.GetMultiStrFlag(flags.TypeFlag), - flags.GetMultiStrFlag(flags.CloudProviderFlag), - flags.GetStrFlag(flags.LibrariesPath)) + c.ScanParams.QueriesPath, + c.ScanParams.Platform, + c.ScanParams.CloudProvider, + c.ScanParams.LibrariesPath) includeQueries := source.IncludeQueries{ ByIDs: []string{queryID}, @@ -191,7 +215,7 @@ func initScan(queryID string) (*engine.Inspector, error) { IncludeQueries: includeQueries, } - t, err := tracker.NewTracker(flags.GetIntFlag(flags.PreviewLinesFlag)) + t, err := tracker.NewTracker(c.ScanParams.PreviewLines) if err != nil { log.Err(err) return &engine.Inspector{}, err @@ -205,7 +229,7 @@ func initScan(queryID string) (*engine.Inspector, error) { t, &queryFilter, make(map[string]bool), - flags.GetIntFlag(flags.QueryExecTimeoutFlag), + c.ScanParams.QueryExecTimeout, false, ) diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index f8f4dc2fba8..5c8d47bacf7 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -1,7 +1,6 @@ package remediation import ( - "fmt" "os" "path/filepath" "regexp" @@ -13,8 +12,8 @@ import ( ) type Summary struct { - SelectedRemediationsNumber int - ActualRemediationsDoneNumber int + SelectedRemediationNumber int + ActualRemediationDoneNumber int } // GetFixs collects all the replacements and additions per file @@ -30,7 +29,7 @@ func (s *Summary) GetFixs(results Result, include []string) map[string]interface var fix Fix if shouldRemediate(&file, include) { - s.SelectedRemediationsNumber++ + s.SelectedRemediationNumber++ r := &Remediation{ Line: file.Line, @@ -79,6 +78,7 @@ func getBefore(line string) string { return string(before[0]) } +// willRemediate verifies if the remediation actually removes the result func willRemediate(remediated []string, originalFileName string, remediation *Remediation) bool { // create temporary file tmpFile := filepath.Join(os.TempDir(), "temporary-remediation-"+utils.NextRandom()+filepath.Ext(originalFileName)) @@ -114,10 +114,37 @@ func removedSimilarityID(results []model.Vulnerability, similarity string) bool result := results[i] if result.SimilarityID == similarity { - fmt.Println(similarity) log.Info().Msgf("failed to remediate '%s'", similarity) return false } } return true } + +// CreateTempFile creates a temporary file with the content as the file pointed in the input +func CreateTempFile(filePath, tmpFilePath string) string { + f, err := os.OpenFile(tmpFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm) + + if err != nil { + f.Close() + log.Error().Msgf("failed to open file '%s': %s", tmpFilePath, err) + return "" + } + + content, err := os.ReadFile(filePath) + + if err != nil { + f.Close() + log.Error().Msgf("failed to read file '%s': %s", filePath, err) + return "" + } + + if _, err = f.Write(content); err != nil { + f.Close() + log.Error().Msgf("failed to write file '%s': %s", tmpFilePath, err) + return "" + } + + f.Close() + return tmpFilePath +} diff --git a/pkg/remediation/utils_test.go b/pkg/remediation/utils_test.go index 664830794e5..40e9250898b 100644 --- a/pkg/remediation/utils_test.go +++ b/pkg/remediation/utils_test.go @@ -1,17 +1,18 @@ package remediation import ( + "os" "path/filepath" "reflect" "testing" + "github.com/Checkmarx/kics/pkg/utils" "github.com/stretchr/testify/require" ) func Test_GetFixs(t *testing.T) { - filePath := filepath.Join("assets", "queries", "terraform", "alicloud", - "ram_account_password_policy_not_required_symbols", "test", "negative1.tf") + filePath := filepath.Join("..", "..", "test", "fixtures", "kics_auto_remediation", "terraform.tf") file1 := &File{ FilePath: filePath, @@ -75,10 +76,10 @@ func Test_GetFixs(t *testing.T) { } tests := []struct { - name string - args args - selectedRemediationsNumber int - want map[string]interface{} + name string + args args + selectedRemediationNumber int + want map[string]interface{} }{ { name: "include all similarityID", @@ -86,8 +87,8 @@ func Test_GetFixs(t *testing.T) { res: res, include: []string{"all"}, }, - want: want, - selectedRemediationsNumber: 2, + want: want, + selectedRemediationNumber: 2, }, { name: "include specific similarityID", @@ -95,21 +96,48 @@ func Test_GetFixs(t *testing.T) { res: res, include: []string{"87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc"}, }, - want: want2, - selectedRemediationsNumber: 1, + want: want2, + selectedRemediationNumber: 1, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := &Summary{ - SelectedRemediationsNumber: 0, - ActualRemediationsDoneNumber: 0, + SelectedRemediationNumber: 0, + ActualRemediationDoneNumber: 0, } if got := s.GetFixs(*tt.args.res, tt.args.include); !reflect.DeepEqual(got, tt.want) { t.Errorf("GetFixs() = %v, want %v", got, tt.want) } - require.Equal(t, s.SelectedRemediationsNumber, tt.selectedRemediationsNumber) + require.Equal(t, s.SelectedRemediationNumber, tt.selectedRemediationNumber) + }) + } +} + +func Test_CreateTempFile(t *testing.T) { + filePath := filepath.Join("..", "..", "test", "fixtures", "kics_auto_remediation", "terraform.tf") + + tmpFile := filepath.Join(os.TempDir(), "temporary-remediation-"+utils.NextRandom()+filepath.Ext(filePath)) + + tests := []struct { + name string + filePath string + tmpFile string + want string + }{ + { + name: "create a temporary file with same content as the input", + filePath: filePath, + tmpFile: tmpFile, + want: tmpFile, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := CreateTempFile(tt.filePath, tt.tmpFile) + require.Equal(t, got, tt.want) }) } } From 7c486aa88bbf73ace9c5c7d423f54edd04081f23 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Fri, 8 Jul 2022 11:08:47 +0100 Subject: [PATCH 08/22] fixing unit test + improving --- e2e/testcases/e2e-cli-057_fix_all.go | 8 +++---- e2e/testcases/e2e-cli-058_fix_include_ids.go | 8 +++---- pkg/remediation/remediation_test.go | 8 +++---- pkg/remediation/utils.go | 8 +++---- pkg/remediation/utils_test.go | 22 ++++++++++---------- 5 files changed, 26 insertions(+), 28 deletions(-) diff --git a/e2e/testcases/e2e-cli-057_fix_all.go b/e2e/testcases/e2e-cli-057_fix_all.go index ee7b0a91cdd..e38b80f986b 100644 --- a/e2e/testcases/e2e-cli-057_fix_all.go +++ b/e2e/testcases/e2e-cli-057_fix_all.go @@ -17,12 +17,12 @@ func init() { // nolint log.Error().Msgf("failed to mkdir: %s", err) } - filePath := "./fixtures/samples/kics-auto-remediation/terraform.tf" - tmpFilePath := "./fixtures/tmp-kics-ar/temporary-remediation" + utils.NextRandom() + filepath.Ext(filePath) + filePathCopyFrom := "./fixtures/samples/kics-auto-remediation/terraform.tf" + tmpFilePath := "./fixtures/tmp-kics-ar/temporary-remediation" + utils.NextRandom() + filepath.Ext(filePathCopyFrom) jsonPath := "./fixtures/tmp-kics-ar" - // create a temporary file with the same content as filePath - tmpFile := remediation.CreateTempFile(filePath, tmpFilePath) + // create a temporary file with the same content as filePathCopyFrom + tmpFile := remediation.CreateTempFile(filePathCopyFrom, tmpFilePath) // create JSON results with remediation generateReport(tmpFile, jsonPath) diff --git a/e2e/testcases/e2e-cli-058_fix_include_ids.go b/e2e/testcases/e2e-cli-058_fix_include_ids.go index b0b727b77c6..97e64927b61 100644 --- a/e2e/testcases/e2e-cli-058_fix_include_ids.go +++ b/e2e/testcases/e2e-cli-058_fix_include_ids.go @@ -17,12 +17,12 @@ func init() { // nolint log.Error().Msgf("failed to mkdir: %s", err) } - filePath := "./fixtures/samples/kics-auto-remediation/terraform.tf" - tmpFilePath := "./fixtures/tmp-kics-ar/temporary-remediation" + utils.NextRandom() + filepath.Ext(filePath) + filePathCopyFrom := "./fixtures/samples/kics-auto-remediation/terraform.tf" + tmpFilePath := "./fixtures/tmp-kics-ar/temporary-remediation" + utils.NextRandom() + filepath.Ext(filePathCopyFrom) jsonPath := "./fixtures/tmp-kics-ar" - // create a temporary file with the same content as filePath - tmpFile := remediation.CreateTempFile(filePath, tmpFilePath) + // create a temporary file with the same content as filePathCopyFrom + tmpFile := remediation.CreateTempFile(filePathCopyFrom, tmpFilePath) // create JSON results with remediation generateReport(tmpFile, jsonPath) diff --git a/pkg/remediation/remediation_test.go b/pkg/remediation/remediation_test.go index bd77b0f6b35..a6eed09fb11 100644 --- a/pkg/remediation/remediation_test.go +++ b/pkg/remediation/remediation_test.go @@ -14,7 +14,7 @@ import ( func Test_RemediateFile(t *testing.T) { - filePath := filepath.Join("..", "..", "test", "fixtures", "kics_auto_remediation", "terraform.tf") + filePathCopyFrom := filepath.Join("..", "..", "test", "fixtures", "kics_auto_remediation", "terraform.tf") type args struct { fix Fix @@ -102,8 +102,6 @@ func Test_RemediateFile(t *testing.T) { source.ListSupportedCloudProviders(), ) - flags.SetMultiStrFlag(flags.QueriesPath, []string{"../../assets/queries"}) - for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := &Summary{ @@ -111,8 +109,8 @@ func Test_RemediateFile(t *testing.T) { ActualRemediationDoneNumber: 0, } - tmpFileName := filepath.Join(os.TempDir(), "temporary-remediation"+utils.NextRandom()+filepath.Ext(filePath)) - tmpFile := CreateTempFile(filePath, tmpFileName) + tmpFileName := filepath.Join(os.TempDir(), "temporary-remediation"+utils.NextRandom()+filepath.Ext(filePathCopyFrom)) + tmpFile := CreateTempFile(filePathCopyFrom, tmpFileName) s.RemediateFile(tmpFile, tt.args.fix) os.Remove(tmpFile) diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index 5c8d47bacf7..988ee804ab6 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -121,8 +121,8 @@ func removedSimilarityID(results []model.Vulnerability, similarity string) bool return true } -// CreateTempFile creates a temporary file with the content as the file pointed in the input -func CreateTempFile(filePath, tmpFilePath string) string { +// CreateTempFile creates a temporary file with the content as the file pointed in the filePathCopyFrom +func CreateTempFile(filePathCopyFrom, tmpFilePath string) string { f, err := os.OpenFile(tmpFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm) if err != nil { @@ -131,11 +131,11 @@ func CreateTempFile(filePath, tmpFilePath string) string { return "" } - content, err := os.ReadFile(filePath) + content, err := os.ReadFile(filePathCopyFrom) if err != nil { f.Close() - log.Error().Msgf("failed to read file '%s': %s", filePath, err) + log.Error().Msgf("failed to read file '%s': %s", filePathCopyFrom, err) return "" } diff --git a/pkg/remediation/utils_test.go b/pkg/remediation/utils_test.go index 40e9250898b..ac154ed3f3b 100644 --- a/pkg/remediation/utils_test.go +++ b/pkg/remediation/utils_test.go @@ -116,27 +116,27 @@ func Test_GetFixs(t *testing.T) { } func Test_CreateTempFile(t *testing.T) { - filePath := filepath.Join("..", "..", "test", "fixtures", "kics_auto_remediation", "terraform.tf") + filePathCopyFrom := filepath.Join("..", "..", "test", "fixtures", "kics_auto_remediation", "terraform.tf") - tmpFile := filepath.Join(os.TempDir(), "temporary-remediation-"+utils.NextRandom()+filepath.Ext(filePath)) + tmpFile := filepath.Join(os.TempDir(), "temporary-remediation-"+utils.NextRandom()+filepath.Ext(filePathCopyFrom)) tests := []struct { - name string - filePath string - tmpFile string - want string + name string + filePathCopyFrom string + tmpFile string + want string }{ { - name: "create a temporary file with same content as the input", - filePath: filePath, - tmpFile: tmpFile, - want: tmpFile, + name: "create a temporary file with same content as the input", + filePathCopyFrom: filePathCopyFrom, + tmpFile: tmpFile, + want: tmpFile, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got := CreateTempFile(tt.filePath, tt.tmpFile) + got := CreateTempFile(tt.filePathCopyFrom, tt.tmpFile) require.Equal(t, got, tt.want) }) } From 181f989febe473b8be39f3ca5b516c249022e796 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Fri, 8 Jul 2022 11:39:59 +0100 Subject: [PATCH 09/22] fix errors --- pkg/engine/inspector.go | 1 + pkg/engine/vulnerability_utils.go | 2 +- pkg/remediation/remediation.go | 7 ++++++- pkg/remediation/utils.go | 3 ++- pkg/remediation/utils_test.go | 4 ++-- 5 files changed, 12 insertions(+), 5 deletions(-) diff --git a/pkg/engine/inspector.go b/pkg/engine/inspector.go index f18e1ccbfd5..81d3fdfb837 100644 --- a/pkg/engine/inspector.go +++ b/pkg/engine/inspector.go @@ -55,6 +55,7 @@ type QueryLoader struct { type VulnerabilityBuilder func(ctx *QueryContext, tracker Tracker, v interface{}, detector *detector.DetectLine) (model.Vulnerability, error) +// PreparedQuery includes the opaQuery and its metadata type PreparedQuery struct { OpaQuery rego.PreparedEvalQuery Metadata model.QueryMetadata diff --git a/pkg/engine/vulnerability_utils.go b/pkg/engine/vulnerability_utils.go index 220825eed47..e833c13f115 100644 --- a/pkg/engine/vulnerability_utils.go +++ b/pkg/engine/vulnerability_utils.go @@ -49,7 +49,7 @@ func mergeWithMetadata(base, additional map[string]interface{}) map[string]inter } func mustMapKeyToString(m map[string]interface{}, key string) *string { res, err := mapKeyToString(m, key, true) - excludedFields := []string{"value", "resourceName", "resourceType"} + excludedFields := []string{"value", "resourceName", "resourceType", "remediation", "remediationType"} if err != nil && !utils.Contains(key, excludedFields) { log.Warn(). Str("reason", err.Error()). diff --git a/pkg/remediation/remediation.go b/pkg/remediation/remediation.go index b6802ecf303..b848a47cab5 100644 --- a/pkg/remediation/remediation.go +++ b/pkg/remediation/remediation.go @@ -9,15 +9,18 @@ import ( "github.com/rs/zerolog/log" ) -type Result struct { +// Report includes all query results +type Report struct { Queries []Query `json:"queries"` } +// Query includes all the files that presents a result related to the queryID type Query struct { Files []File `json:"files"` QueryID string `json:"query_id"` } +// File presents the result information related to the file type File struct { FilePath string `json:"file_name"` Line int `json:"line"` @@ -26,6 +29,7 @@ type File struct { SimilarityID string `json:"similarity_id"` } +// Remediation presents all the relevant information for the fix type Remediation struct { Line int Remediation string @@ -33,6 +37,7 @@ type Remediation struct { QueryID string } +// Fix includes all the replacements and additions related to a file type Fix struct { Replacement []Remediation Addition []Remediation diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index 988ee804ab6..5bdf8381128 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -11,13 +11,14 @@ import ( "github.com/rs/zerolog/log" ) +// Summary represents the information about the number of selected remediation and remediation done type Summary struct { SelectedRemediationNumber int ActualRemediationDoneNumber int } // GetFixs collects all the replacements and additions per file -func (s *Summary) GetFixs(results Result, include []string) map[string]interface{} { +func (s *Summary) GetFixs(results Report, include []string) map[string]interface{} { fixs := make(map[string]interface{}) for i := range results.Queries { diff --git a/pkg/remediation/utils_test.go b/pkg/remediation/utils_test.go index ac154ed3f3b..8fc77d06787 100644 --- a/pkg/remediation/utils_test.go +++ b/pkg/remediation/utils_test.go @@ -40,7 +40,7 @@ func Test_GetFixs(t *testing.T) { QueryID: "41a38329-d81b-4be4-aef4-55b2615d3282", } - res := &Result{ + res := &Report{ Queries: []Query{*query1, *query2}, } @@ -71,7 +71,7 @@ func Test_GetFixs(t *testing.T) { want2[filePath] = fix2 type args struct { - res *Result + res *Report include []string } From af7b10efdb0310f43632ba958c1751c8621a6070 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Fri, 8 Jul 2022 11:58:23 +0100 Subject: [PATCH 10/22] fix --- internal/console/fix.go | 2 +- pkg/remediation/remediation.go | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/console/fix.go b/internal/console/fix.go index 64278f77009..28ea8c71dfb 100644 --- a/internal/console/fix.go +++ b/internal/console/fix.go @@ -84,7 +84,7 @@ func fix(cmd *cobra.Command) error { return err } - results := remediation.Result{} + results := remediation.Report{} err = json.Unmarshal(content, &results) if err != nil { diff --git a/pkg/remediation/remediation.go b/pkg/remediation/remediation.go index b848a47cab5..d814989d10b 100644 --- a/pkg/remediation/remediation.go +++ b/pkg/remediation/remediation.go @@ -84,6 +84,7 @@ func (s *Summary) RemediateFile(filePath string, fix Fix) error { return nil } +// ReplacementInfo presents the relevant information to do the replacement type ReplacementInfo struct { Before string `json:"before"` After string `json:"after"` From 48a94c4b23ecc9f7681862b933a181eaef4ed8e7 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Fri, 8 Jul 2022 12:09:25 +0100 Subject: [PATCH 11/22] correcting f.Close --- pkg/remediation/utils.go | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index 5bdf8381128..910a4360150 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -87,15 +87,20 @@ func willRemediate(remediated []string, originalFileName string, remediation *Re if err != nil { log.Error().Msgf("failed to open temporary file for remediation '%s': %s", remediation.SimilarityID, err) - f.Close() return false } content := []byte(strings.Join(remediated, "\n")) + defer func(f *os.File) { + err = f.Close() + if err != nil { + log.Err(err).Msgf("failed to close file: %s", tmpFile) + } + }(f) + if _, err = f.Write(content); err != nil { log.Error().Msgf("failed to write temporary file for remediation '%s': %s", remediation.SimilarityID, err) - f.Close() return false } @@ -127,25 +132,28 @@ func CreateTempFile(filePathCopyFrom, tmpFilePath string) string { f, err := os.OpenFile(tmpFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm) if err != nil { - f.Close() log.Error().Msgf("failed to open file '%s': %s", tmpFilePath, err) return "" } content, err := os.ReadFile(filePathCopyFrom) + defer func(f *os.File) { + err = f.Close() + if err != nil { + log.Err(err).Msgf("failed to close file: %s", tmpFilePath) + } + }(f) + if err != nil { - f.Close() log.Error().Msgf("failed to read file '%s': %s", filePathCopyFrom, err) return "" } if _, err = f.Write(content); err != nil { - f.Close() log.Error().Msgf("failed to write file '%s': %s", tmpFilePath, err) return "" } - f.Close() return tmpFilePath } From 52dd94f255da3b2e2c57b7ecb0bb3e1ac2a9891e Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Fri, 8 Jul 2022 12:23:19 +0100 Subject: [PATCH 12/22] improving --- internal/console/fix.go | 3 +++ pkg/remediation/remediation.go | 2 ++ pkg/remediation/utils.go | 2 ++ 3 files changed, 7 insertions(+) diff --git a/internal/console/fix.go b/internal/console/fix.go index 28ea8c71dfb..326d657fd61 100644 --- a/internal/console/fix.go +++ b/internal/console/fix.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "os" + "path/filepath" "github.com/Checkmarx/kics/internal/console/flags" consoleHelpers "github.com/Checkmarx/kics/internal/console/helpers" @@ -78,6 +79,8 @@ func fix(cmd *cobra.Command) error { resultsPath := flags.GetStrFlag(flags.Results) include := flags.GetMultiStrFlag(flags.IncludeIds) + filepath.Clean(resultsPath) + content, err := os.ReadFile(resultsPath) if err != nil { log.Error().Msgf("failed to read file: %s", err) diff --git a/pkg/remediation/remediation.go b/pkg/remediation/remediation.go index d814989d10b..bfcc4d35016 100644 --- a/pkg/remediation/remediation.go +++ b/pkg/remediation/remediation.go @@ -3,6 +3,7 @@ package remediation import ( "encoding/json" "os" + "path/filepath" "sort" "strings" @@ -45,6 +46,7 @@ type Fix struct { // RemediateFile remediates the replacements first and secondly, the additions sorted down func (s *Summary) RemediateFile(filePath string, fix Fix) error { + filepath.Clean(filePath) content, err := os.ReadFile(filePath) if err != nil { diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index 910a4360150..02b54558c9a 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -129,6 +129,8 @@ func removedSimilarityID(results []model.Vulnerability, similarity string) bool // CreateTempFile creates a temporary file with the content as the file pointed in the filePathCopyFrom func CreateTempFile(filePathCopyFrom, tmpFilePath string) string { + filepath.Clean(filePathCopyFrom) + filepath.Clean(tmpFilePath) f, err := os.OpenFile(tmpFilePath, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm) if err != nil { From 65ce7768d4cb51e75e67ddb2eb8e0c28ec314075 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Fri, 8 Jul 2022 17:54:20 +0100 Subject: [PATCH 13/22] improving --- e2e/testcases/e2e-cli-057_fix_all.go | 18 ++++++++++++++---- e2e/testcases/e2e-cli-058_fix_include_ids.go | 18 ++++++++++++++---- pkg/remediation/utils.go | 1 + 3 files changed, 29 insertions(+), 8 deletions(-) diff --git a/e2e/testcases/e2e-cli-057_fix_all.go b/e2e/testcases/e2e-cli-057_fix_all.go index e38b80f986b..aacc4d30183 100644 --- a/e2e/testcases/e2e-cli-057_fix_all.go +++ b/e2e/testcases/e2e-cli-057_fix_all.go @@ -13,13 +13,23 @@ import ( // E2E-CLI-057 - Kics fix command // should fix all remediation found func init() { // nolint - if err := os.MkdirAll("./fixtures/tmp-kics-ar", os.ModePerm); err != nil { + cwd, err := os.Getwd() + + if err != nil { + log.Error().Msgf("failed to get wd: %s", err) + } + + tmpFolderPath := filepath.Join(cwd, "fixtures", "tmp-kics-ar") + + if err := os.MkdirAll(tmpFolderPath, os.ModePerm); err != nil { log.Error().Msgf("failed to mkdir: %s", err) } - filePathCopyFrom := "./fixtures/samples/kics-auto-remediation/terraform.tf" - tmpFilePath := "./fixtures/tmp-kics-ar/temporary-remediation" + utils.NextRandom() + filepath.Ext(filePathCopyFrom) - jsonPath := "./fixtures/tmp-kics-ar" + filePathCopyFrom := filepath.Join(cwd, "fixtures", "samples", "kics-auto-remediation", "terraform.tf") + + tmpFilePath := filepath.Join(cwd, "fixtures", "tmp-kics-ar", "temporary-remediation-"+utils.NextRandom()+filepath.Ext(filePathCopyFrom)) + + jsonPath := tmpFolderPath // create a temporary file with the same content as filePathCopyFrom tmpFile := remediation.CreateTempFile(filePathCopyFrom, tmpFilePath) diff --git a/e2e/testcases/e2e-cli-058_fix_include_ids.go b/e2e/testcases/e2e-cli-058_fix_include_ids.go index 97e64927b61..43e137692ab 100644 --- a/e2e/testcases/e2e-cli-058_fix_include_ids.go +++ b/e2e/testcases/e2e-cli-058_fix_include_ids.go @@ -13,13 +13,23 @@ import ( // E2E-CLI-058 - Kics fix command with include-ids flag // should fix the recomendations pointed in include-ids flag func init() { // nolint - if err := os.MkdirAll("./fixtures/tmp-kics-ar", os.ModePerm); err != nil { + cwd, err := os.Getwd() + + if err != nil { + log.Error().Msgf("failed to get wd: %s", err) + } + + tmpFolderPath := filepath.Join(cwd, "fixtures", "tmp-kics-ar") + + if err := os.MkdirAll(tmpFolderPath, os.ModePerm); err != nil { log.Error().Msgf("failed to mkdir: %s", err) } - filePathCopyFrom := "./fixtures/samples/kics-auto-remediation/terraform.tf" - tmpFilePath := "./fixtures/tmp-kics-ar/temporary-remediation" + utils.NextRandom() + filepath.Ext(filePathCopyFrom) - jsonPath := "./fixtures/tmp-kics-ar" + filePathCopyFrom := filepath.Join(cwd, "fixtures", "samples", "kics-auto-remediation", "terraform.tf") + + tmpFilePath := filepath.Join(cwd, "fixtures", "tmp-kics-ar", "temporary-remediation-"+utils.NextRandom()+filepath.Ext(filePathCopyFrom)) + + jsonPath := tmpFolderPath // create a temporary file with the same content as filePathCopyFrom tmpFile := remediation.CreateTempFile(filePathCopyFrom, tmpFilePath) diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index 02b54558c9a..16b6b57da20 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -81,6 +81,7 @@ func getBefore(line string) string { // willRemediate verifies if the remediation actually removes the result func willRemediate(remediated []string, originalFileName string, remediation *Remediation) bool { + filepath.Clean(originalFileName) // create temporary file tmpFile := filepath.Join(os.TempDir(), "temporary-remediation-"+utils.NextRandom()+filepath.Ext(originalFileName)) f, err := os.OpenFile(tmpFile, os.O_WRONLY|os.O_CREATE|os.O_TRUNC, os.ModePerm) From 0c09dd877f1d79eda23341d34082b1f39ad4a441 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Mon, 11 Jul 2022 08:28:41 +0100 Subject: [PATCH 14/22] fixing E2E --- e2e/testcases/e2e-cli-057_fix_all.go | 33 +-------------- e2e/testcases/e2e-cli-058_fix_include_ids.go | 37 ++-------------- e2e/testcases/utils.go | 44 +++++++++++++++++++- 3 files changed, 48 insertions(+), 66 deletions(-) diff --git a/e2e/testcases/e2e-cli-057_fix_all.go b/e2e/testcases/e2e-cli-057_fix_all.go index aacc4d30183..1bdff4d2afa 100644 --- a/e2e/testcases/e2e-cli-057_fix_all.go +++ b/e2e/testcases/e2e-cli-057_fix_all.go @@ -1,48 +1,19 @@ package testcases import ( - "os" - "path/filepath" "regexp" - - "github.com/Checkmarx/kics/pkg/remediation" - "github.com/Checkmarx/kics/pkg/utils" - "github.com/rs/zerolog/log" ) // E2E-CLI-057 - Kics fix command // should fix all remediation found func init() { // nolint - cwd, err := os.Getwd() - - if err != nil { - log.Error().Msgf("failed to get wd: %s", err) - } - - tmpFolderPath := filepath.Join(cwd, "fixtures", "tmp-kics-ar") - - if err := os.MkdirAll(tmpFolderPath, os.ModePerm); err != nil { - log.Error().Msgf("failed to mkdir: %s", err) - } - - filePathCopyFrom := filepath.Join(cwd, "fixtures", "samples", "kics-auto-remediation", "terraform.tf") - - tmpFilePath := filepath.Join(cwd, "fixtures", "tmp-kics-ar", "temporary-remediation-"+utils.NextRandom()+filepath.Ext(filePathCopyFrom)) - - jsonPath := tmpFolderPath - - // create a temporary file with the same content as filePathCopyFrom - tmpFile := remediation.CreateTempFile(filePathCopyFrom, tmpFilePath) - - // create JSON results with remediation - generateReport(tmpFile, jsonPath) - pathResults := filepath.Join(jsonPath, "results.json") + generateResults("results-fix-all") testSample := TestCase{ Name: "should fix all remediation found [E2E-CLI-057]", Args: args{ Args: []cmdArgs{ - []string{"fix", "--results", pathResults, "-v"}, + []string{"fix", "--results", "/path/e2e/fixtures/tmp-kics-ar/results-fix-all.json", "-v"}, }, }, WantStatus: []int{0}, diff --git a/e2e/testcases/e2e-cli-058_fix_include_ids.go b/e2e/testcases/e2e-cli-058_fix_include_ids.go index 43e137692ab..0c24e685d3d 100644 --- a/e2e/testcases/e2e-cli-058_fix_include_ids.go +++ b/e2e/testcases/e2e-cli-058_fix_include_ids.go @@ -1,48 +1,19 @@ package testcases import ( - "os" - "path/filepath" "regexp" - - "github.com/Checkmarx/kics/pkg/remediation" - "github.com/Checkmarx/kics/pkg/utils" - "github.com/rs/zerolog/log" ) -// E2E-CLI-058 - Kics fix command with include-ids flag -// should fix the recomendations pointed in include-ids flag +// E2E-CLI-057 - Kics fix command +// should fix all remediation found func init() { // nolint - cwd, err := os.Getwd() - - if err != nil { - log.Error().Msgf("failed to get wd: %s", err) - } - - tmpFolderPath := filepath.Join(cwd, "fixtures", "tmp-kics-ar") - - if err := os.MkdirAll(tmpFolderPath, os.ModePerm); err != nil { - log.Error().Msgf("failed to mkdir: %s", err) - } - - filePathCopyFrom := filepath.Join(cwd, "fixtures", "samples", "kics-auto-remediation", "terraform.tf") - - tmpFilePath := filepath.Join(cwd, "fixtures", "tmp-kics-ar", "temporary-remediation-"+utils.NextRandom()+filepath.Ext(filePathCopyFrom)) - - jsonPath := tmpFolderPath - - // create a temporary file with the same content as filePathCopyFrom - tmpFile := remediation.CreateTempFile(filePathCopyFrom, tmpFilePath) - - // create JSON results with remediation - generateReport(tmpFile, jsonPath) - pathResults := filepath.Join(jsonPath, "results.json") + generateResults("results-fix-include-ids") testSample := TestCase{ Name: "should fix the recomendations pointed in include-ids flag [E2E-CLI-058]", Args: args{ Args: []cmdArgs{ - []string{"fix", "--results", pathResults, + []string{"fix", "--results", "/path/e2e/fixtures/tmp-kics-ar/results-fix-include-ids.json", "--include-ids", "f282fa13cf5e4ffd4bbb0ee2059f8d0240edcd2ca54b3bb71633145d961de5ce," + "87abbee5d0ec977ba193371c702dca2c040ea902d2e606806a63b66119ff89bc", "-v"}, diff --git a/e2e/testcases/utils.go b/e2e/testcases/utils.go index f35aa05feff..013fb008033 100644 --- a/e2e/testcases/utils.go +++ b/e2e/testcases/utils.go @@ -1,12 +1,18 @@ package testcases import ( + "os" + "path/filepath" + + u "github.com/Checkmarx/kics/e2e/utils" "github.com/Checkmarx/kics/pkg/model" + "github.com/Checkmarx/kics/pkg/remediation" "github.com/Checkmarx/kics/pkg/report" + "github.com/Checkmarx/kics/pkg/utils" "github.com/rs/zerolog/log" ) -func generateReport(tmpFile, jsonPath string) { // nolint +func generateReport(tmpFile, jsonPath, reportName string) { // nolint var queryHigh = model.QueryResult{ QueryName: "Ram Account Password Policy Not Required Minimum Length", QueryID: "a9dfec39-a740-4105-bbd6-721ba163c053", @@ -167,9 +173,43 @@ func generateReport(tmpFile, jsonPath string) { // nolint } // create JSON report - err := report.PrintJSONReport(jsonPath, "results", summary) + err := report.PrintJSONReport(jsonPath, reportName, summary) if err != nil { log.Error().Msgf("failed to create JSON report: %s", err) } } + +func generateResults(reportName string) { + cwd, err := os.Getwd() + + if err != nil { + log.Error().Msgf("failed to get wd: %s", err) + } + + tmpFolderPath := filepath.Join(cwd, "fixtures", "tmp-kics-ar") + + if err := os.MkdirAll(tmpFolderPath, os.ModePerm); err != nil { + log.Error().Msgf("failed to mkdir: %s", err) + } + + filePathCopyFrom := filepath.Join(cwd, "fixtures", "samples", "kics-auto-remediation", "terraform.tf") + + tmpFileName := "temporary-remediation-" + utils.NextRandom() + filepath.Ext(filePathCopyFrom) + tmpFilePath := filepath.Join(cwd, "fixtures", "tmp-kics-ar", tmpFileName) + + jsonPath := tmpFolderPath + + // create a temporary file with the same content as filePathCopyFrom + tmpFile := remediation.CreateTempFile(filePathCopyFrom, tmpFilePath) + + kicsDockerImage := u.GetKICSDockerImageName() + useDocker := kicsDockerImage != "" + + if useDocker { + tmpFile = "/path/e2e/fixtures/tmp-kics-ar/" + tmpFileName + } + + // create JSON results with remediation + generateReport(tmpFile, jsonPath, reportName) +} From 970c2ad2d6fb482cd445029c1a90888860e64b96 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Mon, 11 Jul 2022 09:49:20 +0100 Subject: [PATCH 15/22] test --- docker/Dockerfile.ubi8 | 1 - 1 file changed, 1 deletion(-) diff --git a/docker/Dockerfile.ubi8 b/docker/Dockerfile.ubi8 index d86ce785d4c..8e00a223e53 100644 --- a/docker/Dockerfile.ubi8 +++ b/docker/Dockerfile.ubi8 @@ -86,7 +86,6 @@ RUN unzip terraform-provider-google_4.10.0_linux_amd64.zip && rm terraform-provi RUN unzip terraform-provider-aws_3.72.0_linux_amd64.zip && rm terraform-provider-aws_3.72.0_linux_amd64.zip RUN mkdir ~/.terraform.d && mkdir ~/.terraform.d/plugins && mkdir ~/.terraform.d/plugins/linux_amd64 && mv terraform-provider-aws_v3.72.0_x5 terraform-provider-google_v4.10.0_x5 terraform-provider-azurerm_v2.95.0_x5 ~/.terraform.d/plugins/linux_amd64 -USER ${KUSER} # Copy built binary to the runtime container COPY --chown=${KUSER}:${KGROUP} --from=build_env /build/bin/kics /app/bin/kics From ae57ab8a0b78f9c40f1f27ce66cf9d50da31eb03 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Mon, 11 Jul 2022 15:54:10 +0100 Subject: [PATCH 16/22] adding more tests --- .../aws/alb_listening_on_http/query.rego | 4 - .../dockerfile/add_instead_of_copy/query.rego | 4 - .../query.rego | 2 +- internal/console/fix.go | 10 +- pkg/remediation/utils.go | 113 ++++++++++++------ test/queries_content_test.go | 16 +++ test/queries_test.go | 60 ++++++++++ 7 files changed, 161 insertions(+), 48 deletions(-) diff --git a/assets/queries/ansible/aws/alb_listening_on_http/query.rego b/assets/queries/ansible/aws/alb_listening_on_http/query.rego index b3bf8349894..c8b5c92e81b 100644 --- a/assets/queries/ansible/aws/alb_listening_on_http/query.rego +++ b/assets/queries/ansible/aws/alb_listening_on_http/query.rego @@ -11,8 +11,6 @@ CxPolicy[result] { applicationLb.listeners[index].Protocol != "HTTPS" - remediation := {"before": applicationLb.listeners[index].Protocol, "after": "HTTPS"} - result := { "documentId": id, "resourceType": modules[m], @@ -21,8 +19,6 @@ CxPolicy[result] { "issueType": "IncorrectValue", "keyExpectedValue": "'aws_elb_application_lb' Protocol should be 'HTTP'", "keyActualValue": "'aws_elb_application_lb' Protocol it's not 'HTTP'", - "remediation": json.marshal(remediation), - "remediationType": "replacement", } } diff --git a/assets/queries/dockerfile/add_instead_of_copy/query.rego b/assets/queries/dockerfile/add_instead_of_copy/query.rego index 6274aecbce8..add0d6e9153 100644 --- a/assets/queries/dockerfile/add_instead_of_copy/query.rego +++ b/assets/queries/dockerfile/add_instead_of_copy/query.rego @@ -8,15 +8,11 @@ CxPolicy[result] { not dockerLib.arrayContains(resource.Value, {".tar", ".tar."}) - remediation := {"before": "ADD", "after": "COPY"} - result := { "documentId": input.document[i].id, "searchKey": sprintf("FROM={{%s}}.{{%s}}", [name, resource.Original]), "issueType": "IncorrectValue", "keyExpectedValue": sprintf("'COPY' %s", [resource.Value[0]]), "keyActualValue": sprintf("'ADD' %s", [resource.Value[0]]), - "remediation": json.marshal(remediation), - "remediationType": "replacement", } } diff --git a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego index 7c54ebd799c..0ab71022797 100644 --- a/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego +++ b/assets/queries/terraform/alicloud/ram_account_password_policy_not_required_minimum_length/query.rego @@ -8,7 +8,7 @@ CxPolicy[result] { resource := input.document[i].resource.alicloud_ram_account_password_policy[name] resource.minimum_password_length < 14 - remediation := {"before": resource.minimum_password_length, "after": "14"} + remediation := {"before": format_int(resource.minimum_password_length, 10), "after": "14"} result := { "documentId": input.document[i].id, diff --git a/internal/console/fix.go b/internal/console/fix.go index 326d657fd61..39b90c3333a 100644 --- a/internal/console/fix.go +++ b/internal/console/fix.go @@ -6,6 +6,7 @@ import ( "fmt" "os" "path/filepath" + "sync" "github.com/Checkmarx/kics/internal/console/flags" consoleHelpers "github.com/Checkmarx/kics/internal/console/helpers" @@ -103,13 +104,20 @@ func fix(cmd *cobra.Command) error { // get all the fixs related to each filePath fixs := summary.GetFixs(results, include) + wg := &sync.WaitGroup{} + for filePath := range fixs { + wg.Add(1) fix := fixs[filePath].(remediation.Fix) - err = summary.RemediateFile(filePath, fix) + go func(filePath string) { + defer wg.Done() + err = summary.RemediateFile(filePath, fix) + }(filePath) if err != nil { return err } } + wg.Wait() fmt.Printf("\nSelected remediation: %d\n", summary.SelectedRemediationNumber) fmt.Printf("Remediation done: %d\n", summary.ActualRemediationDoneNumber) diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index 16b6b57da20..ac6556d66ae 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -21,51 +21,17 @@ type Summary struct { func (s *Summary) GetFixs(results Report, include []string) map[string]interface{} { fixs := make(map[string]interface{}) - for i := range results.Queries { - query := results.Queries[i] - - for j := range query.Files { - file := query.Files[j] - - var fix Fix - - if shouldRemediate(&file, include) { - s.SelectedRemediationNumber++ - - r := &Remediation{ - Line: file.Line, - Remediation: file.Remediation, - SimilarityID: file.SimilarityID, - QueryID: query.QueryID, - } - if file.RemediationType == "replacement" { - fix.Replacement = append(fix.Replacement, *r) - } - - if file.RemediationType == "addition" { - fix.Addition = append(fix.Addition, *r) - } - - if _, ok := fixs[file.FilePath]; !ok { - fixs[file.FilePath] = fix - continue - } - - updatedFix := fixs[file.FilePath].(Fix) - - updatedFix.Addition = append(updatedFix.Addition, fix.Addition...) - updatedFix.Replacement = append(updatedFix.Replacement, fix.Replacement...) + vulns := getVulns(results) - fixs[file.FilePath] = updatedFix - } - } + if len(vulns) > 0 { + fixs = s.GetFixsFromVulns(vulns, include) } return fixs } func shouldRemediate(file *File, include []string) bool { - if len(file.Remediation) > 0 && (include[0] == "all" || utils.Contains(file.SimilarityID, include)) { + if len(file.Remediation) > 0 && len(file.RemediationType) > 0 && (include[0] == "all" || utils.Contains(file.SimilarityID, include)) { return true } @@ -160,3 +126,74 @@ func CreateTempFile(filePathCopyFrom, tmpFilePath string) string { return tmpFilePath } + +func (s *Summary) GetFixsFromVulns(vulnerabilities []model.Vulnerability, include []string) map[string]interface{} { + fixs := make(map[string]interface{}) + + for i := range vulnerabilities { + vuln := vulnerabilities[i] + + file := File{ + FilePath: vuln.FileName, + Line: vuln.Line, + Remediation: vuln.Remediation, + RemediationType: vuln.RemediationType, + SimilarityID: vuln.SimilarityID, + } + + var fix Fix + + if shouldRemediate(&file, include) { + s.SelectedRemediationNumber++ + r := &Remediation{ + Line: file.Line, + Remediation: file.Remediation, + SimilarityID: file.SimilarityID, + QueryID: vuln.QueryID, + } + + if file.RemediationType == "replacement" { + fix.Replacement = append(fix.Replacement, *r) + } + + if file.RemediationType == "addition" { + fix.Addition = append(fix.Addition, *r) + } + + if _, ok := fixs[file.FilePath]; !ok { + fixs[file.FilePath] = fix + continue + } + + updatedFix := fixs[file.FilePath].(Fix) + + updatedFix.Addition = append(updatedFix.Addition, fix.Addition...) + updatedFix.Replacement = append(updatedFix.Replacement, fix.Replacement...) + + fixs[file.FilePath] = updatedFix + } + } + return fixs +} + +func getVulns(results Report) []model.Vulnerability { + vulns := []model.Vulnerability{} + for i := range results.Queries { + query := results.Queries[i] + + for j := range query.Files { + file := query.Files[j] + vuln := &model.Vulnerability{ + FileName: file.FilePath, + Line: file.Line, + Remediation: file.Remediation, + RemediationType: file.RemediationType, + SimilarityID: file.SimilarityID, + QueryID: query.QueryID, + } + + vulns = append(vulns, *vuln) + } + } + return vulns +} diff --git a/test/queries_content_test.go b/test/queries_content_test.go index e2db9b50dec..08dc00b4ab2 100644 --- a/test/queries_content_test.go +++ b/test/queries_content_test.go @@ -236,6 +236,22 @@ func testQueryHasGoodReturnParams(t *testing.T, entry queryEntry) { //nolint )) } + if _, ok := m["remediation"]; ok { + _, ok := m["remediationType"] + require.True(t, ok, fmt.Sprintf( + "query '%s' doesn't include parameter 'remediationType' in response", + path.Base(entry.dir))) + + } + + if _, ok := m["remediationType"]; ok { + _, ok := m["remediation"] + require.True(t, ok, fmt.Sprintf( + "query '%s' doesn't include parameter 'remediation' in response", + path.Base(entry.dir))) + + } + return model.Vulnerability{}, nil }, trk, diff --git a/test/queries_test.go b/test/queries_test.go index 90a6a040999..c4c8d43f0b0 100644 --- a/test/queries_test.go +++ b/test/queries_test.go @@ -12,6 +12,7 @@ import ( "sync" "testing" + "github.com/Checkmarx/kics/internal/console/flags" "github.com/Checkmarx/kics/internal/constants" "github.com/Checkmarx/kics/internal/tracker" "github.com/Checkmarx/kics/pkg/engine" @@ -19,9 +20,12 @@ import ( "github.com/Checkmarx/kics/pkg/engine/source" "github.com/Checkmarx/kics/pkg/model" "github.com/Checkmarx/kics/pkg/progress" + "github.com/Checkmarx/kics/pkg/remediation" + "github.com/Checkmarx/kics/pkg/utils" "github.com/golang/mock/gomock" "github.com/rs/zerolog" "github.com/rs/zerolog/log" + "github.com/spf13/cobra" "github.com/stretchr/testify/require" ) @@ -50,6 +54,58 @@ func TestQueries(t *testing.T) { } } +func testRemediationQuery(t testing.TB, entry queryEntry, vulnerabilities []model.Vulnerability) { + summary := &remediation.Summary{ + SelectedRemediationNumber: 0, + ActualRemediationDoneNumber: 0, + } + + // get fixs from query vulns + fixs := summary.GetFixsFromVulns(vulnerabilities, []string{"all"}) + + if len(fixs) > 0 { + // verify if the remediations vulns actually fixes the results + data, err := os.ReadFile(filepath.FromSlash("../internal/console/assets/scan-flags.json")) + require.NoError(t, err) + + mockCmd := &cobra.Command{ + Use: "mock", + Short: "Mock cmd", + RunE: func(cmd *cobra.Command, args []string) error { + return nil + }, + } + + flags.InitJSONFlags( + mockCmd, + string(data), + true, + source.ListSupportedPlatforms(), + source.ListSupportedCloudProviders(), + ) + + temporaryFixs := make(map[string]interface{}) + + for k := range fixs { + tmpFilePath := filepath.Join(os.TempDir(), "temporary-remediation-"+utils.NextRandom()+filepath.Ext(k)) + tmpFile := remediation.CreateTempFile(k, tmpFilePath) + + temporaryFixs[tmpFile] = fixs[k] + } + + for filePath := range temporaryFixs { + fix := temporaryFixs[filePath].(remediation.Fix) + + err = summary.RemediateFile(filePath, fix) + os.Remove(filePath) + require.NoError(t, err) + } + + require.Equal(t, summary.SelectedRemediationNumber, summary.ActualRemediationDoneNumber, + "'SelectedRemediationNumber' is different from 'ActualRemediationDoneNumber'") + } +} + func TestUniqueQueryIDs(t *testing.T) { log.Logger = log.Output(zerolog.ConsoleWriter{Out: io.Discard}) queries := loadQueries(t) @@ -199,6 +255,10 @@ func testQuery(tb testing.TB, entry queryEntry, filesPath []string, expectedVuln require.Nil(tb, err) validateQueryResultFields(tb, vulnerabilities) requireEqualVulnerabilities(tb, expectedVulnerabilities, vulnerabilities, entry.dir) + + if entry.platform == "terraform" { + testRemediationQuery(tb, entry, vulnerabilities) + } } func vulnerabilityCompare(vulnerabilitySlice []model.Vulnerability, i, j int) bool { From 4e69405e5aa430d0d38f35895ca14a6aa66ad575 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Tue, 12 Jul 2022 09:42:05 +0100 Subject: [PATCH 17/22] fixing codacy issue --- pkg/remediation/utils.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index ac6556d66ae..0d69087d816 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -127,6 +127,7 @@ func CreateTempFile(filePathCopyFrom, tmpFilePath string) string { return tmpFilePath } +// GetFixsFromVulns collects all the replacements and additions per file from []model.Vulnerability func (s *Summary) GetFixsFromVulns(vulnerabilities []model.Vulnerability, include []string) map[string]interface{} { fixs := make(map[string]interface{}) From f672e20cb5dcc84949faae208e2d48c43a6d9071 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Tue, 12 Jul 2022 15:55:34 +0100 Subject: [PATCH 18/22] improving tests --- pkg/parser/parser.go | 2 ++ pkg/remediation/remediation.go | 2 ++ pkg/remediation/scan.go | 2 ++ pkg/remediation/utils.go | 8 ++++++++ test/queries_test.go | 16 +++++++++++++--- 5 files changed, 27 insertions(+), 3 deletions(-) diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index d8137647aae..0e4738249c4 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -5,6 +5,7 @@ import ( "errors" "os" "strings" + "sync" "github.com/Checkmarx/kics/pkg/model" "github.com/Checkmarx/kics/pkg/utils" @@ -72,6 +73,7 @@ type Parser struct { parsers kindParser extensions model.Extensions Platform []string + Mu sync.RWMutex } // ParsedDocument is a struct containing data retrieved from parsing diff --git a/pkg/remediation/remediation.go b/pkg/remediation/remediation.go index bfcc4d35016..40961a34f23 100644 --- a/pkg/remediation/remediation.go +++ b/pkg/remediation/remediation.go @@ -154,7 +154,9 @@ func (s *Summary) writeRemediation(remediatedLines, lines []string, filePath, si } log.Info().Msgf("file '%s' was remediated with '%s'", filePath, similarityID) + s.mu.Lock() s.ActualRemediationDoneNumber++ + s.mu.Unlock() return remediatedLines } diff --git a/pkg/remediation/scan.go b/pkg/remediation/scan.go index 00117ef9fc4..6cb87f9afe8 100644 --- a/pkg/remediation/scan.go +++ b/pkg/remediation/scan.go @@ -114,7 +114,9 @@ func getPayload(filePath string, content []byte) (model.FileMetadatas, error) { return model.FileMetadatas{}, errors.New("failed to get parser") } + p[0].Mu.Lock() documents, er := p[0].Parse(filePath, content) + p[0].Mu.Unlock() if er != nil { log.Error().Msgf("failed to parse file '%s': %s", filePath, er) diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index 0d69087d816..572049bc05b 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -5,6 +5,7 @@ import ( "path/filepath" "regexp" "strings" + "sync" "github.com/Checkmarx/kics/pkg/model" "github.com/Checkmarx/kics/pkg/utils" @@ -15,6 +16,7 @@ import ( type Summary struct { SelectedRemediationNumber int ActualRemediationDoneNumber int + mu sync.RWMutex } // GetFixs collects all the replacements and additions per file @@ -71,8 +73,12 @@ func willRemediate(remediated []string, originalFileName string, remediation *Re return false } + m := sync.RWMutex{} + + m.Lock() // scan the temporary file to verify if the remediation removed the result results, err := scanTmpFile(tmpFile, remediation.QueryID, content) + m.Unlock() if err != nil { log.Error().Msgf("failed to get results of query %s: %s", remediation.QueryID, err) @@ -145,7 +151,9 @@ func (s *Summary) GetFixsFromVulns(vulnerabilities []model.Vulnerability, includ var fix Fix if shouldRemediate(&file, include) { + s.mu.Lock() s.SelectedRemediationNumber++ + s.mu.Unlock() r := &Remediation{ Line: file.Line, Remediation: file.Remediation, diff --git a/test/queries_test.go b/test/queries_test.go index c4c8d43f0b0..e0476ba6e5d 100644 --- a/test/queries_test.go +++ b/test/queries_test.go @@ -93,16 +93,26 @@ func testRemediationQuery(t testing.TB, entry queryEntry, vulnerabilities []mode temporaryFixs[tmpFile] = fixs[k] } + wg := &sync.WaitGroup{} + for filePath := range temporaryFixs { + wg.Add(1) fix := temporaryFixs[filePath].(remediation.Fix) - err = summary.RemediateFile(filePath, fix) - os.Remove(filePath) - require.NoError(t, err) + go func(filePath string) { + defer wg.Done() + err = summary.RemediateFile(filePath, fix) + os.Remove(filePath) + if err != nil { + require.NoError(t, err) + } + }(filePath) } + wg.Wait() require.Equal(t, summary.SelectedRemediationNumber, summary.ActualRemediationDoneNumber, "'SelectedRemediationNumber' is different from 'ActualRemediationDoneNumber'") + } } From 94aaabab15d1f4368de8dbc4c5af2caf9312fac5 Mon Sep 17 00:00:00 2001 From: rafaela-soares Date: Tue, 12 Jul 2022 17:32:38 +0100 Subject: [PATCH 19/22] testing permissions on Dockerfile.ubi8 --- docker/Dockerfile.ubi8 | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docker/Dockerfile.ubi8 b/docker/Dockerfile.ubi8 index 8e00a223e53..8f0e75e2a04 100644 --- a/docker/Dockerfile.ubi8 +++ b/docker/Dockerfile.ubi8 @@ -86,11 +86,14 @@ RUN unzip terraform-provider-google_4.10.0_linux_amd64.zip && rm terraform-provi RUN unzip terraform-provider-aws_3.72.0_linux_amd64.zip && rm terraform-provider-aws_3.72.0_linux_amd64.zip RUN mkdir ~/.terraform.d && mkdir ~/.terraform.d/plugins && mkdir ~/.terraform.d/plugins/linux_amd64 && mv terraform-provider-aws_v3.72.0_x5 terraform-provider-google_v4.10.0_x5 terraform-provider-azurerm_v2.95.0_x5 ~/.terraform.d/plugins/linux_amd64 +USER ${KUSER} # Copy built binary to the runtime container COPY --chown=${KUSER}:${KGROUP} --from=build_env /build/bin/kics /app/bin/kics COPY --chown=${KUSER}:${KGROUP} --from=build_env /build/assets/ /app/bin/assets/ +RUN chmod 700 /app/bin/kics + ENV PATH $PATH:/app/bin # Command to run the executable From adc3ba8e9a9309b4217e36b3fb77ce3d13edc283 Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Thu, 14 Jul 2022 14:59:33 +0100 Subject: [PATCH 20/22] Merge branch 'kics_auto_remediation/terraform_alic --- e2e/testcases/e2e-cli-060_fix_text.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/e2e/testcases/e2e-cli-060_fix_text.go b/e2e/testcases/e2e-cli-060_fix_text.go index 588af7ce843..f2f5b670402 100644 --- a/e2e/testcases/e2e-cli-060_fix_text.go +++ b/e2e/testcases/e2e-cli-060_fix_text.go @@ -7,11 +7,7 @@ func init() { //nolint Name: "should display an error regarding missing --results flag [E2E-CLI-060]", Args: args{ Args: []cmdArgs{ -<<<<<<< HEAD - []string{"fix"}, -======= []string{"remediate"}, ->>>>>>> master }, ExpectedOut: []string{"E2E_CLI_060"}, }, From e94041f0cfcd492f5adc6aadd885b8f85e541859 Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Thu, 14 Jul 2022 15:04:31 +0100 Subject: [PATCH 21/22] remove changes --- docker/Dockerfile.ubi8 | 2 - docs/commands.md | 1 - docs/kics-auto-remediation.md | 30 ------ e2e/fixtures/assets/fix_help | 18 ---- e2e/fixtures/assets/help | 1 - internal/console/assets/fix-flags.json | 15 --- internal/console/fix.go | 131 ------------------------- internal/console/flags/fix_flags.go | 7 -- 8 files changed, 205 deletions(-) delete mode 100644 docs/kics-auto-remediation.md delete mode 100644 e2e/fixtures/assets/fix_help delete mode 100644 internal/console/assets/fix-flags.json delete mode 100644 internal/console/fix.go delete mode 100644 internal/console/flags/fix_flags.go diff --git a/docker/Dockerfile.ubi8 b/docker/Dockerfile.ubi8 index 8f0e75e2a04..d86ce785d4c 100644 --- a/docker/Dockerfile.ubi8 +++ b/docker/Dockerfile.ubi8 @@ -92,8 +92,6 @@ USER ${KUSER} COPY --chown=${KUSER}:${KGROUP} --from=build_env /build/bin/kics /app/bin/kics COPY --chown=${KUSER}:${KGROUP} --from=build_env /build/assets/ /app/bin/assets/ -RUN chmod 700 /app/bin/kics - ENV PATH $PATH:/app/bin # Command to run the executable diff --git a/docs/commands.md b/docs/commands.md index f27f06684c4..d43b2eadcd2 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -13,7 +13,6 @@ Usage: kics [command] Available Commands: - fix Auto remediates the project generate-id Generates uuid for query help Help about any command list-platforms List supported platforms diff --git a/docs/kics-auto-remediation.md b/docs/kics-auto-remediation.md deleted file mode 100644 index 7c85f52dc92..00000000000 --- a/docs/kics-auto-remediation.md +++ /dev/null @@ -1,30 +0,0 @@ -# KICS AUTO REMEDIATION - -With this new feature, KICS provides auto remediation for simple replacements and simple additions in a single line. - -Note that this feature will be only available for Terraform, for now. - -

-image -

- - - -## HOW KICS AR WORKS? - - -1. As a first step, you will need to scan your project/file and generate a JSON report. - - Example: ```docker run -v /home/cosmicgirl/:/path/ kics scan -p /path/sample.tf -i "41a38329-d81b-4be4-aef4-55b2615d3282,a9dfec39-a740-4105-bbd6-721ba163c053,2bb13841-7575-439e-8e0a-cccd9ede2fa8" --no-progress -o /path/results --report-formats json``` - - If KICS makes available a remediation for a result, the result will have the fields `remediation` and `remediation_type` defined. As an example, please see: -

- image -

- - -2. If your JSON report has any result with remediation, you will need to run the new KICS command: fix. - - If you want KICS to remediate all the remediation, you can run ```docker run -v /home/cosmicgirl/:/path/ kics fix --results /path/results/results.json -v```. - - If you want to specify which remediation KICS should fix, you can use the flag `--include-ids`. In this flag, you should point the `similarity_id` of the result. For example: ```docker run -v /home/cosmicgirl/:/path/ kics fix --results /path/results/results.json --include-ids "f282fa13cf5e4ffd4bbb0ee2059f8d0240edcd2ca54b3bb71633145d961de5ce" -v``` diff --git a/e2e/fixtures/assets/fix_help b/e2e/fixtures/assets/fix_help deleted file mode 100644 index c6da4a60582..00000000000 --- a/e2e/fixtures/assets/fix_help +++ /dev/null @@ -1,18 +0,0 @@ -Usage: - kics fix [flags] - -Flags: - -h, --help help for fix - --include-ids strings which remediation (similarity ids) should be remediated - example "f6b7acac2d541d8c15c88d2be51b0e6abd576750b71c580f2e3a9346f7ed0e67,6af5fc5d7c0ad0077348a090f7c09949369d24d5608bbdbd14376a15de62afd1" (default [all]) - --results string points to the JSON results file with remediation - -Global Flags: - --ci display only log messages to CLI output (mutually exclusive with silent) - -f, --log-format string determines log format (pretty,json) (default "pretty") - --log-level string determines log level (TRACE,DEBUG,INFO,WARN,ERROR,FATAL) (default "INFO") - --log-path string path to generate log file (info.log) - --no-color disable CLI color output - --profiling string enables performance profiler that prints resource consumption metrics in the logs during the execution (CPU, MEM) - -s, --silent silence stdout messages (mutually exclusive with verbose and ci) - -v, --verbose write logs to stdout too (mutually exclusive with silent) diff --git a/e2e/fixtures/assets/help b/e2e/fixtures/assets/help index 93df4d5812c..73c4d250ec0 100644 --- a/e2e/fixtures/assets/help +++ b/e2e/fixtures/assets/help @@ -2,7 +2,6 @@ Usage: kics [command] Available Commands: - fix Auto remediates the project generate-id Generates uuid for query help Help about any command list-platforms List supported platforms diff --git a/internal/console/assets/fix-flags.json b/internal/console/assets/fix-flags.json deleted file mode 100644 index 5c21e5b726f..00000000000 --- a/internal/console/assets/fix-flags.json +++ /dev/null @@ -1,15 +0,0 @@ -{ - "include-ids": { - "flagType": "multiStr", - "shorthandFlag": "", - "defaultValue": "all", - "usage": "which remediation (similarity ids) should be remediated \nexample \"f6b7acac2d541d8c15c88d2be51b0e6abd576750b71c580f2e3a9346f7ed0e67,6af5fc5d7c0ad0077348a090f7c09949369d24d5608bbdbd14376a15de62afd1\"" - }, - "results": { - "flagType": "str", - "shorthandFlag": "", - "defaultValue": "", - "usage": "points to the JSON results file with remediation" - } - } - \ No newline at end of file diff --git a/internal/console/fix.go b/internal/console/fix.go deleted file mode 100644 index 39b90c3333a..00000000000 --- a/internal/console/fix.go +++ /dev/null @@ -1,131 +0,0 @@ -package console - -import ( - _ "embed" // Embed fix flags - "encoding/json" - "fmt" - "os" - "path/filepath" - "sync" - - "github.com/Checkmarx/kics/internal/console/flags" - consoleHelpers "github.com/Checkmarx/kics/internal/console/helpers" - sentryReport "github.com/Checkmarx/kics/internal/sentry" - "github.com/Checkmarx/kics/pkg/engine/source" - internalPrinter "github.com/Checkmarx/kics/pkg/printer" - "github.com/Checkmarx/kics/pkg/remediation" - "github.com/pkg/errors" - "github.com/rs/zerolog/log" - "github.com/spf13/cobra" -) - -var ( - //go:embed assets/fix-flags.json - fixFlagsListContent string -) - -// NewFixCmd creates a new instance of the fix Command -func NewFixCmd() *cobra.Command { - return &cobra.Command{ - Use: "fix", - Short: "Auto remediates the project", - PersistentPreRunE: func(cmd *cobra.Command, args []string) error { - return preFix(cmd) - }, - RunE: func(cmd *cobra.Command, args []string) error { - return fix(cmd) - }, - } -} - -func initFixCmd(fixCmd *cobra.Command) error { - if err := flags.InitJSONFlags( - fixCmd, - fixFlagsListContent, - false, - source.ListSupportedPlatforms(), - source.ListSupportedCloudProviders()); err != nil { - return err - } - - if err := fixCmd.MarkFlagRequired(flags.Results); err != nil { - sentryReport.ReportSentry(&sentryReport.Report{ - Message: "Failed to add command required flags", - Err: err, - Location: "func initScanCmd()", - }, true) - log.Err(err).Msg("Failed to add command required flags") - } - return nil -} - -func preFix(cmd *cobra.Command) error { - err := flags.Validate() - if err != nil { - return err - } - - err = flags.ValidateQuerySelectionFlags() - if err != nil { - return err - } - err = internalPrinter.SetupPrinter(cmd.InheritedFlags()) - if err != nil { - return errors.New(initError + err.Error()) - } - return err -} - -func fix(cmd *cobra.Command) error { - resultsPath := flags.GetStrFlag(flags.Results) - include := flags.GetMultiStrFlag(flags.IncludeIds) - - filepath.Clean(resultsPath) - - content, err := os.ReadFile(resultsPath) - if err != nil { - log.Error().Msgf("failed to read file: %s", err) - return err - } - - results := remediation.Report{} - - err = json.Unmarshal(content, &results) - if err != nil { - log.Error().Msgf("failed to unmarshal file: %s", err) - return err - } - - summary := &remediation.Summary{ - SelectedRemediationNumber: 0, - ActualRemediationDoneNumber: 0, - } - - // get all the fixs related to each filePath - fixs := summary.GetFixs(results, include) - - wg := &sync.WaitGroup{} - - for filePath := range fixs { - wg.Add(1) - fix := fixs[filePath].(remediation.Fix) - go func(filePath string) { - defer wg.Done() - err = summary.RemediateFile(filePath, fix) - }(filePath) - if err != nil { - return err - } - } - wg.Wait() - - fmt.Printf("\nSelected remediation: %d\n", summary.SelectedRemediationNumber) - fmt.Printf("Remediation done: %d\n", summary.ActualRemediationDoneNumber) - - exitCode := consoleHelpers.FixExitCode(summary.SelectedRemediationNumber, summary.ActualRemediationDoneNumber) - if exitCode != 0 { - os.Exit(exitCode) - } - - return nil -} diff --git a/internal/console/flags/fix_flags.go b/internal/console/flags/fix_flags.go deleted file mode 100644 index b90c4324929..00000000000 --- a/internal/console/flags/fix_flags.go +++ /dev/null @@ -1,7 +0,0 @@ -package flags - -// Flags constants for fix -const ( - Results = "results" - IncludeIds = "include-ids" -) From 682fe992f44b689c0a31a2e0078e376fdf6e8338 Mon Sep 17 00:00:00 2001 From: cxMiguelSilva Date: Thu, 14 Jul 2022 15:13:46 +0100 Subject: [PATCH 22/22] delete newline at file end --- e2e/fixtures/E2E_CLI_059 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/fixtures/E2E_CLI_059 b/e2e/fixtures/E2E_CLI_059 index cffeb670b13..eacc766dfd3 100644 --- a/e2e/fixtures/E2E_CLI_059 +++ b/e2e/fixtures/E2E_CLI_059 @@ -1,3 +1,3 @@ Auto remediates the project -{{.RemediateHelp}} +{{.RemediateHelp}} \ No newline at end of file