From 5c361f0f9238377f0679accb3132cc501fef8a82 Mon Sep 17 00:00:00 2001 From: EduardoSemanas Date: Tue, 5 Mar 2024 15:42:29 +0000 Subject: [PATCH 1/5] Detail corrected on scan_test --- pkg/scan/scan_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/scan/scan_test.go b/pkg/scan/scan_test.go index 8a3dff35670..2c6e534a822 100644 --- a/pkg/scan/scan_test.go +++ b/pkg/scan/scan_test.go @@ -26,7 +26,7 @@ func Test_ExecuteScan(t *testing.T) { scanParams: Parameters{ ExcludePaths: []string{ "./../../test/fixtures/test_scan_cloudfront_logging_disabled/metadata.json", - "./../../test/fixtures/test_scan_cloudfront_logging_disabled/positive_expected_result.tf", + "./../../test/fixtures/test_scan_cloudfront_logging_disabled/test/positive_expected_result.tf", }, Path: []string{"./../../test/fixtures/test_scan_cloudfront_logging_disabled/test/positive1.yaml"}, QueriesPath: []string{"./../../test/fixtures/test_scan_cloudfront_logging_disabled"}, From 784a800ff8b6f56ac587ea3b56232de7dbe23f07 Mon Sep 17 00:00:00 2001 From: EduardoSemanas Date: Tue, 5 Mar 2024 15:43:15 +0000 Subject: [PATCH 2/5] Corrected resolver behaviour to actually return errors when caught --- pkg/kics/resolver_sink.go | 2 +- pkg/resolver/resolver.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pkg/kics/resolver_sink.go b/pkg/kics/resolver_sink.go index de005b8a4eb..c88714e96d0 100644 --- a/pkg/kics/resolver_sink.go +++ b/pkg/kics/resolver_sink.go @@ -24,7 +24,7 @@ func (s *Service) resolverSink(ctx context.Context, filename, scanID string, ope resFiles, err := s.Resolver.Resolve(filename, kind) if err != nil { log.Err(err).Msgf("failed to render file content") - return []string{}, nil + return []string{}, err } for _, rfile := range resFiles.File { diff --git a/pkg/resolver/resolver.go b/pkg/resolver/resolver.go index 7ec2db7c15e..1d278553e1c 100644 --- a/pkg/resolver/resolver.go +++ b/pkg/resolver/resolver.go @@ -59,7 +59,7 @@ func (r *Resolver) Resolve(filePath string, kind model.FileKind) (model.Resolved if r, ok := r.resolvers[kind]; ok { obj, err := r.Resolve(filePath) if err != nil { - return model.ResolvedFiles{}, nil + return model.ResolvedFiles{}, err } log.Debug().Msgf("resolver.Resolve() rendered file: %s", filePath) return obj, nil From 36f6da1320a016984744c9a1d4075930da4febd1 Mon Sep 17 00:00:00 2001 From: EduardoSemanas Date: Tue, 5 Mar 2024 15:43:42 +0000 Subject: [PATCH 3/5] Resolver Sink Unit Tests created --- pkg/kics/resolver_sink_test.go | 184 ++++++++++++++++++ .../test/.helmignore | 23 +++ .../test/Chart.yaml | 24 +++ .../test/templates/service.yaml | 16 ++ .../test/values.yaml | 98 ++++++++++ 5 files changed, 345 insertions(+) create mode 100644 pkg/kics/resolver_sink_test.go create mode 100644 test/fixtures/helm_template_parser_error/test/.helmignore create mode 100644 test/fixtures/helm_template_parser_error/test/Chart.yaml create mode 100644 test/fixtures/helm_template_parser_error/test/templates/service.yaml create mode 100644 test/fixtures/helm_template_parser_error/test/values.yaml diff --git a/pkg/kics/resolver_sink_test.go b/pkg/kics/resolver_sink_test.go new file mode 100644 index 00000000000..78a90b7e154 --- /dev/null +++ b/pkg/kics/resolver_sink_test.go @@ -0,0 +1,184 @@ +package kics + +import ( + "context" + "github.com/Checkmarx/kics/assets" + "github.com/Checkmarx/kics/internal/storage" + "github.com/Checkmarx/kics/internal/tracker" + "github.com/Checkmarx/kics/pkg/engine" + "github.com/Checkmarx/kics/pkg/engine/provider" + "github.com/Checkmarx/kics/pkg/engine/secrets" + "github.com/Checkmarx/kics/pkg/engine/source" + "github.com/Checkmarx/kics/pkg/parser" + yamlParser "github.com/Checkmarx/kics/pkg/parser/yaml" + "github.com/Checkmarx/kics/pkg/resolver" + "github.com/Checkmarx/kics/pkg/resolver/helm" + "github.com/rs/zerolog/log" + "github.com/stretchr/testify/require" + "testing" +) + +func Test_ResolverSink(t *testing.T) { + ctx := context.Background() + tests := []struct { + name string + path string + service Service + expectedExcludedCount int + }{ + { + name: "test resolver sink", + path: "./../../test/fixtures/helm_ignore/test", + service: MockService( + []string{"./../../test/fixtures/helm_ignore/test"}, + []string{"Kubernetes"}, + []string{}, + 10, + 60, + 100, + ctx), + expectedExcludedCount: 11, + }, + { + name: "test resolver sink no helm", + path: "./../../test/fixtures/test_scan_cloudfront_logging_disabled", + service: MockService( + []string{"./../../test/fixtures/test_scan_cloudfront_logging_disabled"}, + []string{"CloudFormation"}, + []string{"aws"}, + 10, + 60, + 100, + ctx), + expectedExcludedCount: 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := tt.service + + excluded, err := s.resolverSink(ctx, tt.path, "", false) + if err != nil { + t.Fatalf(`ResolverSink failed for path %s with error: %v`, tt.path, err) + } + + require.Equal(t, tt.expectedExcludedCount, len(excluded)) + }) + } +} + +func Test_ResolverSink_ParseError(t *testing.T) { + ctx := context.Background() + tests := []struct { + name string + path string + service Service + expectedErrorString string + }{ + { + name: "test resolver sink", + path: "./../../test/fixtures/helm_template_parser_error/test", + service: MockService( + []string{"./../../test/fixtures/helm_template_parser_error/test"}, + []string{"Kubernetes"}, + []string{}, + 10, + 60, + 100, + ctx), + expectedErrorString: "failed to render helm chart", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + s := tt.service + _, err := s.resolverSink(ctx, tt.path, "", false) + require.EqualError(t, err, tt.expectedErrorString) + }) + } +} + +func MockService(paths []string, + platforms []string, + cloudProviders []string, + previewLines int, + queryExecTimeout int, + maxFileSizeFlag int, + ctx context.Context) Service { + + path := paths[0] + + filesSource, err := provider.NewFileSystemSourceProvider(paths, []string{}) + if err != nil { + log.Error().Msgf(`Failed to get File Sources for path %s with error: %v`, path, err) + } + + querySource := source.NewFilesystemSource( + []string{}, + platforms, + cloudProviders, + "", + false) + + combinedParser, err := parser.NewBuilder(). + Add(&yamlParser.Parser{}). + Build(querySource.Types, querySource.CloudProviders) + if err != nil { + log.Error().Msgf(`Failed to build parser for path %s with error: %v`, path, err) + } + + mockTracker, err := tracker.NewTracker(previewLines) + if err != nil { + log.Error().Msgf(`Failed to build tracker for path %s with error: %v`, path, err) + } + + queryFilter := source.QueryInspectorParameters{} + + inspector, err := engine.NewInspector(ctx, + querySource, + engine.DefaultVulnerabilityBuilder, + mockTracker, + &queryFilter, + map[string]bool{}, + queryExecTimeout, + true, + 1, + ) + if err != nil { + log.Error().Msgf(`Failed to build inspector for path %s with error: %v`, path, err) + } + + regexRulesContent := assets.SecretsQueryRegexRulesJSON + + secretsInspector, err := secrets.NewInspector( + ctx, + map[string]bool{}, + mockTracker, + &queryFilter, + false, + queryExecTimeout, + regexRulesContent, + false, + ) + if err != nil { + log.Error().Msgf(`Failed to build secretsInspector with error: %v`, err) + } + + mockResolver, err := resolver.NewBuilder().Add(&helm.Resolver{}).Build() + if err != nil { + log.Error().Msgf(`Failed to build mockResolver with error: %v`, err) + } + + return Service{ + SourceProvider: filesSource, + Storage: storage.NewMemoryStorage(), + Parser: combinedParser[0], + Inspector: inspector, + SecretsInspector: secretsInspector, + Tracker: mockTracker, + MaxFileSize: maxFileSizeFlag, + Resolver: mockResolver, + } +} diff --git a/test/fixtures/helm_template_parser_error/test/.helmignore b/test/fixtures/helm_template_parser_error/test/.helmignore new file mode 100644 index 00000000000..0e8a0eb36f4 --- /dev/null +++ b/test/fixtures/helm_template_parser_error/test/.helmignore @@ -0,0 +1,23 @@ +# Patterns to ignore when building packages. +# This supports shell glob matching, relative path matching, and +# negation (prefixed with !). Only one pattern per line. +.DS_Store +# Common VCS dirs +.git/ +.gitignore +.bzr/ +.bzrignore +.hg/ +.hgignore +.svn/ +# Common backup files +*.swp +*.bak +*.tmp +*.orig +*~ +# Various IDEs +.project +.idea/ +*.tmproj +.vscode/ diff --git a/test/fixtures/helm_template_parser_error/test/Chart.yaml b/test/fixtures/helm_template_parser_error/test/Chart.yaml new file mode 100644 index 00000000000..3ebad470cb4 --- /dev/null +++ b/test/fixtures/helm_template_parser_error/test/Chart.yaml @@ -0,0 +1,24 @@ +apiVersion: v2 +name: test +description: A Helm chart for Kubernetes + +# A chart can be either an 'application' or a 'library' chart. +# +# Application charts are a collection of templates that can be packaged into versioned archives +# to be deployed. +# +# Library charts provide useful utilities or functions for the chart developer. They're included as +# a dependency of application charts to inject those utilities and functions into the rendering +# pipeline. Library charts do not define any templates and therefore cannot be deployed. +type: application + +# This is the chart version. This version number should be incremented each time you make changes +# to the chart and its templates, including the app version. +# Versions are expected to follow Semantic Versioning (https://semver.org/) +version: 0.1.0 + +# This is the version number of the application being deployed. This version number should be +# incremented each time you make changes to the application. Versions are not expected to +# follow Semantic Versioning. They should reflect the version the application is using. +# It is recommended to use it with quotes. +appVersion: "1.16.0" diff --git a/test/fixtures/helm_template_parser_error/test/templates/service.yaml b/test/fixtures/helm_template_parser_error/test/templates/service.yaml new file mode 100644 index 00000000000..7b709959635 --- /dev/null +++ b/test/fixtures/helm_template_parser_error/test/templates/service.yaml @@ -0,0 +1,16 @@ +apiVersion: v1 +kind: Service +metadata: + name: {{ include "test.fullname" . }} + labels: + {{- include "test.labels" . | nindent 4 }} +- +spec: + type: {{ .Values.service.type }} + ports: + - port: {{ .Values.service.port }} + targetPort: http + protocol: TCP + name: http + selector: + {{- include "test.selectorLabels" . | nindent 4 }} diff --git a/test/fixtures/helm_template_parser_error/test/values.yaml b/test/fixtures/helm_template_parser_error/test/values.yaml new file mode 100644 index 00000000000..f3cc6241180 --- /dev/null +++ b/test/fixtures/helm_template_parser_error/test/values.yaml @@ -0,0 +1,98 @@ +# Default values for test. +# This is a YAML-formatted file. +# Declare variables to be passed into your templates. + +replicaCount: 1 + +image: + repository: nginx + pullPolicy: IfNotPresent + # Overrides the image tag whose default is the chart appVersion. + tag: "" + +imagePullSecrets: [] +nameOverride: "" +fullnameOverride: "" + +serviceAccount: + # Specifies whether a service account should be created + create: true + # Automatically mount a ServiceAccount's API credentials? + automount: true + # Annotations to add to the service account + annotations: {} + # The name of the service account to use. + # If not set and create is true, a name is generated using the fullname template + name: "" + +podAnnotations: {} +podLabels: {} + +podSecurityContext: {} + # fsGroup: 2000 + +securityContext: {} + # capabilities: + # drop: + # - ALL + # readOnlyRootFilesystem: true + # runAsNonRoot: true + # runAsUser: 1000 + +service: + type: ClusterIP + port: 80 + +ingress: + enabled: false + className: "" + annotations: {} + # kubernetes.io/ingress.class: nginx + # kubernetes.io/tls-acme: "true" + hosts: + - host: chart-example.local + paths: + - path: / + pathType: ImplementationSpecific + tls: [] + # - secretName: chart-example-tls + # hosts: + # - chart-example.local + +resources: {} + # We usually recommend not to specify default resources and to leave this as a conscious + # choice for the user. This also increases chances charts run on environments with little + # resources, such as Minikube. If you do want to specify resources, uncomment the following + # lines, adjust them as necessary, and remove the curly braces after 'resources:'. + # limits: + # cpu: 100m + # memory: 128Mi + # requests: + # cpu: 100m + # memory: 128Mi + +autoscaling: + enabled: false + minReplicas: 1 + maxReplicas: 100 + targetCPUUtilizationPercentage: 80 + # targetMemoryUtilizationPercentage: 80 + +# Additional volumes on the output Deployment definition. +volumes: [] +# - name: foo +# secret: +# secretName: mysecret +# optional: false + +# Additional volumeMounts on the output Deployment definition. +volumeMounts: [] +# - name: foo +# mountPath: "/etc/foo" +# readOnly: true + +nodeSelector: {} + +tolerations: [] + +affinity: {} From fbddf1cb3ed7f6ee5cde581ff41b90ece8062cac Mon Sep 17 00:00:00 2001 From: EduardoSemanas Date: Tue, 5 Mar 2024 15:50:38 +0000 Subject: [PATCH 4/5] Removed redundant character escapes --- pkg/kics/resolver_sink.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/kics/resolver_sink.go b/pkg/kics/resolver_sink.go index c88714e96d0..5bf40b1653e 100644 --- a/pkg/kics/resolver_sink.go +++ b/pkg/kics/resolver_sink.go @@ -101,7 +101,7 @@ func (s *Service) resolverSink(ctx context.Context, filename, scanID string, ope func (s *Service) getOriginalIgnoreLines(filename string, originalFile []uint8, openAPIResolveReferences, isMinified bool) (ignoreLines []int, err error) { - refactor := regexp.MustCompile(`.*\n?.*KICS\_HELM\_ID.+\n`).ReplaceAll(originalFile, []uint8{}) + refactor := regexp.MustCompile(`.*\n?.*KICS_HELM_ID.+\n`).ReplaceAll(originalFile, []uint8{}) refactor = regexp.MustCompile(`{{-\s*(.*?)\s*}}`).ReplaceAll(refactor, []uint8{}) documentsOriginal, err := s.Parser.Parse(filename, refactor, openAPIResolveReferences, isMinified) From 610a50a97748166073368812f076ec2b54828215 Mon Sep 17 00:00:00 2001 From: EduardoSemanas Date: Wed, 6 Mar 2024 10:58:50 +0000 Subject: [PATCH 5/5] Terraformer UTs updated --- pkg/terraformer/terraformer_test.go | 23 +++++++++++++++++++ .../terraformer-output.txt | 1 + 2 files changed, 24 insertions(+) create mode 100644 test/fixtures/terraformer_output_save_test/terraformer-output.txt diff --git a/pkg/terraformer/terraformer_test.go b/pkg/terraformer/terraformer_test.go index ad62e77425c..e7ab502d9eb 100644 --- a/pkg/terraformer/terraformer_test.go +++ b/pkg/terraformer/terraformer_test.go @@ -120,3 +120,26 @@ func Test_BuildArgs(t *testing.T) { }) } } + +func Test_SaveTerraformerOutput(t *testing.T) { + + tests := []struct { + name string + output string + destination string + expectedOutput error + }{ + { + name: "test aws build", + output: "Some Output", + destination: "./../../test/fixtures/terraformer_output_save_test", + expectedOutput: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + v := saveTerraformerOutput(tt.destination, tt.output) + require.Equal(t, tt.expectedOutput, v) + }) + } +} diff --git a/test/fixtures/terraformer_output_save_test/terraformer-output.txt b/test/fixtures/terraformer_output_save_test/terraformer-output.txt new file mode 100644 index 00000000000..ede4c7c5128 --- /dev/null +++ b/test/fixtures/terraformer_output_save_test/terraformer-output.txt @@ -0,0 +1 @@ +Some Output \ No newline at end of file