From 1dbf3fddde60722762320527a981b840e4cc37cf Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Fri, 3 May 2024 16:02:22 +0100 Subject: [PATCH 01/14] Add max resolver depth flag --- internal/console/assets/scan-flags.json | 6 ++++ internal/console/flags/scan_flags.go | 1 + internal/console/pre_scan.go | 1 + internal/console/remediate.go | 3 +- internal/console/scan.go | 1 + pkg/kics/resolver_sink.go | 11 +++---- pkg/kics/resolver_sink_test.go | 4 +-- pkg/kics/service.go | 5 ++-- pkg/kics/sink.go | 5 ++-- pkg/parser/ansible/ini/config/parser.go | 2 +- pkg/parser/ansible/ini/hosts/parser.go | 2 +- pkg/parser/buildah/parser.go | 2 +- pkg/parser/buildah/parser_test.go | 5 ++-- pkg/parser/docker/parser.go | 2 +- pkg/parser/docker/parser_test.go | 2 +- pkg/parser/grpc/parser.go | 2 +- pkg/parser/grpc/parser_test.go | 2 +- pkg/parser/json/parser.go | 4 +-- pkg/parser/json/parser_test.go | 2 +- pkg/parser/parser.go | 6 ++-- pkg/parser/parser_test.go | 8 ++--- pkg/parser/terraform/terraform.go | 2 +- pkg/parser/terraform/terraform_test.go | 2 +- pkg/parser/yaml/parser.go | 4 +-- pkg/parser/yaml/parser_test.go | 2 +- pkg/remediation/remediation.go | 6 ++-- pkg/remediation/remediation_test.go | 2 +- pkg/remediation/scan.go | 8 ++--- pkg/remediation/utils.go | 4 +-- pkg/resolver/file/file.go | 40 ++++++++++++------------- pkg/resolver/file/file_test.go | 9 +++--- pkg/scan/client.go | 1 + pkg/scan/scan.go | 2 +- pkg/scanner/scanner.go | 3 +- test/main_test.go | 2 +- test/queries_test.go | 2 +- 36 files changed, 91 insertions(+), 74 deletions(-) diff --git a/internal/console/assets/scan-flags.json b/internal/console/assets/scan-flags.json index e8c339cd12d..2c4d0ff0500 100644 --- a/internal/console/assets/scan-flags.json +++ b/internal/console/assets/scan-flags.json @@ -227,5 +227,11 @@ "shorthandFlag": "", "defaultValue": "false", "usage": "uses old severities in query results" + }, + "max-resolver-depth": { + "flagType": "int", + "shorthandFlag": "", + "defaultValue": "15", + "usage": "maximum depth to which the resolver will traverse to resolve files" } } diff --git a/internal/console/flags/scan_flags.go b/internal/console/flags/scan_flags.go index f97df4e7961..8b7399abe69 100644 --- a/internal/console/flags/scan_flags.go +++ b/internal/console/flags/scan_flags.go @@ -38,4 +38,5 @@ const ( ParallelScanFile = "parallel" MaxFileSizeFlag = "max-file-size" UseOldSeveritiesFlag = "old-severities" + MaxResolverDepth = "max-resolver-depth" ) diff --git a/internal/console/pre_scan.go b/internal/console/pre_scan.go index 6ee4d2426c6..e45ae9da3cc 100644 --- a/internal/console/pre_scan.go +++ b/internal/console/pre_scan.go @@ -158,6 +158,7 @@ func (console *console) preScan() { log.Info().Msgf("CPU: %.1f", cpu) log.Info().Msgf("Max file size permitted for scanning: %d MB", flags.GetIntFlag(flags.MaxFileSizeFlag)) + log.Info().Msgf("Max resolver depth permitted for resolving files: %d", flags.GetIntFlag(flags.MaxResolverDepth)) noProgress := flags.GetBoolFlag(flags.NoProgressFlag) if strings.EqualFold(flags.GetStrFlag(flags.LogLevelFlag), "debug") { diff --git a/internal/console/remediate.go b/internal/console/remediate.go index ffe7dbc7f3a..f6014e7b4b3 100644 --- a/internal/console/remediate.go +++ b/internal/console/remediate.go @@ -79,6 +79,7 @@ func remediate() error { resultsPath := flags.GetStrFlag(flags.Results) include := flags.GetMultiStrFlag(flags.IncludeIds) openAPIResolveReferences := flags.GetBoolFlag(flags.OpenAPIReferencesFlag) + maxResolverDepth := flags.GetIntFlag(flags.MaxResolverDepth) filepath.Clean(resultsPath) @@ -106,7 +107,7 @@ func remediate() error { for filePath := range remediationSets { fix := remediationSets[filePath].(remediation.Set) - err = summary.RemediateFile(filePath, fix, openAPIResolveReferences) + err = summary.RemediateFile(filePath, fix, openAPIResolveReferences, maxResolverDepth) if err != nil { return err } diff --git a/internal/console/scan.go b/internal/console/scan.go index 06d7e3212df..697b48ad66c 100644 --- a/internal/console/scan.go +++ b/internal/console/scan.go @@ -143,6 +143,7 @@ func getScanParameters(changedDefaultQueryPath, changedDefaultLibrariesPath bool ParallelScanFlag: flags.GetIntFlag(flags.ParallelScanFile), MaxFileSizeFlag: flags.GetIntFlag(flags.MaxFileSizeFlag), UseOldSeverities: flags.GetBoolFlag(flags.UseOldSeveritiesFlag), + MaxResolverDepth: flags.GetIntFlag(flags.MaxResolverDepth), } return &scanParams diff --git a/pkg/kics/resolver_sink.go b/pkg/kics/resolver_sink.go index 5bf40b1653e..54f48c8e4de 100644 --- a/pkg/kics/resolver_sink.go +++ b/pkg/kics/resolver_sink.go @@ -16,7 +16,7 @@ import ( "github.com/rs/zerolog/log" ) -func (s *Service) resolverSink(ctx context.Context, filename, scanID string, openAPIResolveReferences bool) ([]string, error) { +func (s *Service) resolverSink(ctx context.Context, filename, scanID string, openAPIResolveReferences bool, maxResolverDepth int) ([]string, error) { kind := s.Resolver.GetType(filename) if kind == model.KindCOMMON { return []string{}, nil @@ -31,7 +31,7 @@ func (s *Service) resolverSink(ctx context.Context, filename, scanID string, ope s.Tracker.TrackFileFound(rfile.FileName) isMinified := minified.IsMinified(rfile.FileName, rfile.Content) - documents, err := s.Parser.Parse(rfile.FileName, rfile.Content, openAPIResolveReferences, isMinified) + documents, err := s.Parser.Parse(rfile.FileName, rfile.Content, openAPIResolveReferences, isMinified, maxResolverDepth) if err != nil { if documents.Kind == "break" { return []string{}, nil @@ -41,7 +41,7 @@ func (s *Service) resolverSink(ctx context.Context, filename, scanID string, ope } if kind == model.KindHELM { - ignoreList, errorIL := s.getOriginalIgnoreLines(rfile.FileName, rfile.OriginalData, openAPIResolveReferences, isMinified) + ignoreList, errorIL := s.getOriginalIgnoreLines(rfile.FileName, rfile.OriginalData, openAPIResolveReferences, isMinified, maxResolverDepth) if errorIL == nil { documents.IgnoreLines = ignoreList @@ -100,11 +100,12 @@ 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) { + openAPIResolveReferences, isMinified bool, + maxResolverDepth int) (ignoreLines []int, err error) { 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) + documentsOriginal, err := s.Parser.Parse(filename, refactor, openAPIResolveReferences, isMinified, maxResolverDepth) if err == nil { ignoreLines = documentsOriginal.IgnoreLines } diff --git a/pkg/kics/resolver_sink_test.go b/pkg/kics/resolver_sink_test.go index de33dce21b5..85c7bb02472 100644 --- a/pkg/kics/resolver_sink_test.go +++ b/pkg/kics/resolver_sink_test.go @@ -59,7 +59,7 @@ func Test_ResolverSink(t *testing.T) { t.Run(tt.name, func(t *testing.T) { s := tt.service - excluded, err := s.resolverSink(ctx, tt.path, "", false) + excluded, err := s.resolverSink(ctx, tt.path, "", false, 15) if err != nil { t.Fatalf(`ResolverSink failed for path %s with error: %v`, tt.path, err) } @@ -95,7 +95,7 @@ func Test_ResolverSink_ParseError(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { s := tt.service - _, err := s.resolverSink(ctx, tt.path, "", false) + _, err := s.resolverSink(ctx, tt.path, "", false, 15) require.EqualError(t, err, tt.expectedErrorString) }) } diff --git a/pkg/kics/service.go b/pkg/kics/service.go index dd7618afd5f..bb0a0a220a7 100644 --- a/pkg/kics/service.go +++ b/pkg/kics/service.go @@ -66,6 +66,7 @@ type Service struct { func (s *Service) PrepareSources(ctx context.Context, scanID string, openAPIResolveReferences bool, + maxResolverDepth int, wg *sync.WaitGroup, errCh chan<- error) { defer wg.Done() // CxSAST query under review @@ -74,10 +75,10 @@ func (s *Service) PrepareSources(ctx context.Context, ctx, s.Parser.SupportedExtensions(), func(ctx context.Context, filename string, rc io.ReadCloser) error { - return s.sink(ctx, filename, scanID, rc, data, openAPIResolveReferences) + return s.sink(ctx, filename, scanID, rc, data, openAPIResolveReferences, maxResolverDepth) }, func(ctx context.Context, filename string) ([]string, error) { // Sink used for resolver files and templates - return s.resolverSink(ctx, filename, scanID, openAPIResolveReferences) + return s.resolverSink(ctx, filename, scanID, openAPIResolveReferences, maxResolverDepth) }, ); err != nil { errCh <- errors.Wrap(err, "failed to read sources") diff --git a/pkg/kics/sink.go b/pkg/kics/sink.go index 37c2459b161..9c67c96d455 100644 --- a/pkg/kics/sink.go +++ b/pkg/kics/sink.go @@ -28,7 +28,8 @@ var ( func (s *Service) sink(ctx context.Context, filename, scanID string, rc io.Reader, data []byte, - openAPIResolveReferences bool) error { + openAPIResolveReferences bool, + maxResolverDepth int) error { s.Tracker.TrackFileFound(filename) log.Debug().Msgf("Starting to process file %s", filename) @@ -42,7 +43,7 @@ func (s *Service) sink(ctx context.Context, filename, scanID string, if err != nil { return errors.Wrapf(err, "failed to get file content: %s", filename) } - documents, err := s.Parser.Parse(filename, *content, openAPIResolveReferences, c.IsMinified) + documents, err := s.Parser.Parse(filename, *content, openAPIResolveReferences, c.IsMinified, maxResolverDepth) if err != nil { log.Err(err).Msgf("failed to parse file content: %s", filename) return nil diff --git a/pkg/parser/ansible/ini/config/parser.go b/pkg/parser/ansible/ini/config/parser.go index 67e9b8be983..1c8cc57b44d 100644 --- a/pkg/parser/ansible/ini/config/parser.go +++ b/pkg/parser/ansible/ini/config/parser.go @@ -13,7 +13,7 @@ import ( type Parser struct { } -func (p *Parser) Resolve(fileContent []byte, _ string, _ bool) ([]byte, error) { +func (p *Parser) Resolve(fileContent []byte, _ string, _ bool, _ int) ([]byte, error) { return fileContent, nil } diff --git a/pkg/parser/ansible/ini/hosts/parser.go b/pkg/parser/ansible/ini/hosts/parser.go index b921062f684..4e229a4bb97 100644 --- a/pkg/parser/ansible/ini/hosts/parser.go +++ b/pkg/parser/ansible/ini/hosts/parser.go @@ -13,7 +13,7 @@ import ( type Parser struct { } -func (p *Parser) Resolve(fileContent []byte, _ string, _ bool) ([]byte, error) { +func (p *Parser) Resolve(fileContent []byte, _ string, _ bool, _ int) ([]byte, error) { return fileContent, nil } diff --git a/pkg/parser/buildah/parser.go b/pkg/parser/buildah/parser.go index 07b1ad67fc8..c06f9e5813a 100644 --- a/pkg/parser/buildah/parser.go +++ b/pkg/parser/buildah/parser.go @@ -51,7 +51,7 @@ const ( ) // Resolve - replace or modifies in-memory content before parsing -func (p *Parser) Resolve(fileContent []byte, _ string, _ bool) ([]byte, error) { +func (p *Parser) Resolve(fileContent []byte, _ string, _ bool, _ int) ([]byte, error) { return fileContent, nil } diff --git a/pkg/parser/buildah/parser_test.go b/pkg/parser/buildah/parser_test.go index 9704a2a9982..e30432c051e 100644 --- a/pkg/parser/buildah/parser_test.go +++ b/pkg/parser/buildah/parser_test.go @@ -2,10 +2,11 @@ package buildah import ( "encoding/json" - "github.com/Checkmarx/kics/pkg/model" "reflect" "testing" + "github.com/Checkmarx/kics/pkg/model" + "github.com/stretchr/testify/require" ) @@ -320,7 +321,7 @@ func TestParser_Resolve(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { p := &Parser{} - got, err := p.Resolve(tt.args.fileContent, tt.args.filename, true) + got, err := p.Resolve(tt.args.fileContent, tt.args.filename, true, 15) if (err != nil) != tt.wantErr { t.Errorf("Parser.Resolve() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/parser/docker/parser.go b/pkg/parser/docker/parser.go index 9e0a2d88582..4dd94544b98 100644 --- a/pkg/parser/docker/parser.go +++ b/pkg/parser/docker/parser.go @@ -34,7 +34,7 @@ type Command struct { } // Resolve - replace or modifies in-memory content before parsing -func (p *Parser) Resolve(fileContent []byte, _ string, _ bool) ([]byte, error) { +func (p *Parser) Resolve(fileContent []byte, _ string, _ bool, _ int) ([]byte, error) { return fileContent, nil } diff --git a/pkg/parser/docker/parser_test.go b/pkg/parser/docker/parser_test.go index 5369f4ecbef..4d136c167f6 100644 --- a/pkg/parser/docker/parser_test.go +++ b/pkg/parser/docker/parser_test.go @@ -149,7 +149,7 @@ func Test_Resolve(t *testing.T) { ENTRYPOINT ["java","-Djava.security.egd=file:/dev/./urandom","-jar","/app.jar"] ` - resolved, err := parser.Resolve([]byte(have), "Dockerfile", true) + resolved, err := parser.Resolve([]byte(have), "Dockerfile", true, 15) require.NoError(t, err) require.Equal(t, []byte(have), resolved) } diff --git a/pkg/parser/grpc/parser.go b/pkg/parser/grpc/parser.go index 97f88b5602d..9a3abf76b8b 100644 --- a/pkg/parser/grpc/parser.go +++ b/pkg/parser/grpc/parser.go @@ -65,7 +65,7 @@ func (p *Parser) StringifyContent(content []byte) (string, error) { } // Resolve resolves proto files variables -func (p *Parser) Resolve(fileContent []byte, _ string, _ bool) ([]byte, error) { +func (p *Parser) Resolve(fileContent []byte, _ string, _ bool, _ int) ([]byte, error) { return fileContent, nil } diff --git a/pkg/parser/grpc/parser_test.go b/pkg/parser/grpc/parser_test.go index a7d35db7ef1..4970eed4e2a 100644 --- a/pkg/parser/grpc/parser_test.go +++ b/pkg/parser/grpc/parser_test.go @@ -527,7 +527,7 @@ func TestParser_Resolve(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { p := &Parser{} - got, err := p.Resolve(tt.args.fileContent, tt.args.filename, true) + got, err := p.Resolve(tt.args.fileContent, tt.args.filename, true, 15) if (err != nil) != tt.wantErr { t.Errorf("Parser.Resolve() error = %v, wantErr %v", err, tt.wantErr) return diff --git a/pkg/parser/json/parser.go b/pkg/parser/json/parser.go index 88ed6215844..7d9acb5952c 100644 --- a/pkg/parser/json/parser.go +++ b/pkg/parser/json/parser.go @@ -15,11 +15,11 @@ type Parser struct { } // Resolve - replace or modifies in-memory content before parsing -func (p *Parser) Resolve(fileContent []byte, filename string, resolveReferences bool) ([]byte, error) { +func (p *Parser) Resolve(fileContent []byte, filename string, resolveReferences bool, maxResolverDepth int) ([]byte, error) { // Resolve files passed as arguments with file resolver (e.g. file://) res := file.NewResolver(json.Unmarshal, json.Marshal, p.SupportedExtensions()) resolvedFilesCache := make(map[string]file.ResolvedFile) - resolved := res.Resolve(fileContent, filename, 0, resolvedFilesCache, resolveReferences) + resolved := res.Resolve(fileContent, filename, 0, maxResolverDepth, resolvedFilesCache, resolveReferences) p.resolvedFiles = res.ResolvedFiles if len(res.ResolvedFiles) == 0 { return fileContent, nil diff --git a/pkg/parser/json/parser_test.go b/pkg/parser/json/parser_test.go index 923ca70111a..f6c124e33f8 100644 --- a/pkg/parser/json/parser_test.go +++ b/pkg/parser/json/parser_test.go @@ -49,7 +49,7 @@ func TestParser_Parse(t *testing.T) { func Test_Resolve(t *testing.T) { parser := &Parser{} - resolved, err := parser.Resolve([]byte(have), "test.json", true) + resolved, err := parser.Resolve([]byte(have), "test.json", true, 15) require.NoError(t, err) require.Equal(t, have, string(resolved)) } diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index d07d2b4e5d7..b252759424e 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -17,7 +17,7 @@ type kindParser interface { SupportedExtensions() []string SupportedTypes() map[string]bool Parse(filePath string, fileContent []byte) ([]model.Document, []int, error) - Resolve(fileContent []byte, filename string, _ bool) ([]byte, error) + Resolve(fileContent []byte, filename string, _ bool, _ int) ([]byte, error) StringifyContent(content []byte) (string, error) GetResolvedFiles() map[string]model.ResolvedFile } @@ -123,11 +123,11 @@ func (c *Parser) CommentsCommands(filePath string, fileContent []byte) model.Com // Parse executes a parser on the fileContent and returns the file content as a Document, the file kind and // an error, if an error has occurred -func (c *Parser) Parse(filePath string, fileContent []byte, openAPIResolveReferences, isMinified bool) (ParsedDocument, error) { +func (c *Parser) Parse(filePath string, fileContent []byte, openAPIResolveReferences, isMinified bool, maxResolverDepth int) (ParsedDocument, error) { fileContent = utils.DecryptAnsibleVault(fileContent, os.Getenv("ANSIBLE_VAULT_PASSWORD_FILE")) if c.isValidExtension(filePath) { - resolved, err := c.parsers.Resolve(fileContent, filePath, openAPIResolveReferences) + resolved, err := c.parsers.Resolve(fileContent, filePath, openAPIResolveReferences, maxResolverDepth) if err != nil { return ParsedDocument{}, err } diff --git a/pkg/parser/parser_test.go b/pkg/parser/parser_test.go index af945354184..df74e5659c8 100644 --- a/pkg/parser/parser_test.go +++ b/pkg/parser/parser_test.go @@ -25,7 +25,7 @@ func TestParser_Parse(t *testing.T) { "name": "CxBraga" } } -`), true, false) +`), true, false, 15) require.NoError(t, err) require.Len(t, docs.Docs, 1) require.Contains(t, docs.Docs[0], "martin") @@ -39,7 +39,7 @@ func TestParser_Parse(t *testing.T) { docs, err := parser.Parse("../../test/fixtures/test_extension/test.yaml", []byte(` martin: name: CxBraga -`), true, false) +`), true, false, 15) require.NoError(t, err) require.Len(t, docs.Docs, 1) require.Contains(t, docs.Docs[0], "martin") @@ -54,7 +54,7 @@ martin: FROM foo COPY . / RUN echo hello -`), true, false) +`), true, false, 15) require.NoError(t, err) require.Len(t, docs.Docs, 1) @@ -70,7 +70,7 @@ func TestParser_Empty(t *testing.T) { t.Errorf("Error building parser: %s", err) } for _, parser := range p { - docs, err := parser.Parse("test.json", nil, true, false) + docs, err := parser.Parse("test.json", nil, true, false, 15) require.Nil(t, docs.Docs) require.Equal(t, model.FileKind(""), docs.Kind) require.Error(t, err) diff --git a/pkg/parser/terraform/terraform.go b/pkg/parser/terraform/terraform.go index 4e7e43e74cd..837919e690b 100644 --- a/pkg/parser/terraform/terraform.go +++ b/pkg/parser/terraform/terraform.go @@ -46,7 +46,7 @@ func NewDefaultWithVarsPath(terraformVarsPath string) *Parser { } // Resolve - replace or modifies in-memory content before parsing -func (p *Parser) Resolve(fileContent []byte, filename string, _ bool) ([]byte, error) { +func (p *Parser) Resolve(fileContent []byte, filename string, _ bool, _ int) ([]byte, error) { // handle panic during resolve process defer func() { if r := recover(); r != nil { diff --git a/pkg/parser/terraform/terraform_test.go b/pkg/parser/terraform/terraform_test.go index 60a164edc56..a879b2e0840 100644 --- a/pkg/parser/terraform/terraform_test.go +++ b/pkg/parser/terraform/terraform_test.go @@ -178,7 +178,7 @@ func Test_namelessResource(t *testing.T) { func Test_Resolve(t *testing.T) { parser := NewDefault() - resolved, err := parser.Resolve([]byte(have), "test.tf", true) + resolved, err := parser.Resolve([]byte(have), "test.tf", true, 15) require.NoError(t, err) require.Equal(t, []byte(have), resolved) } diff --git a/pkg/parser/yaml/parser.go b/pkg/parser/yaml/parser.go index fd5738b8b06..be73f3cbb3e 100644 --- a/pkg/parser/yaml/parser.go +++ b/pkg/parser/yaml/parser.go @@ -18,11 +18,11 @@ type Parser struct { } // Resolve - replace or modifies in-memory content before parsing -func (p *Parser) Resolve(fileContent []byte, filename string, resolveReferences bool) ([]byte, error) { +func (p *Parser) Resolve(fileContent []byte, filename string, resolveReferences bool, maxResolverDepth int) ([]byte, error) { // Resolve files passed as arguments with file resolver (e.g. file://) res := file.NewResolver(yaml.Unmarshal, yaml.Marshal, p.SupportedExtensions()) resolvedFilesCache := make(map[string]file.ResolvedFile) - resolved := res.Resolve(fileContent, filename, 0, resolvedFilesCache, resolveReferences) + resolved := res.Resolve(fileContent, filename, 0, maxResolverDepth, resolvedFilesCache, resolveReferences) p.resolvedFiles = res.ResolvedFiles if len(res.ResolvedFiles) == 0 { return fileContent, nil diff --git a/pkg/parser/yaml/parser_test.go b/pkg/parser/yaml/parser_test.go index f674cdfcba9..c3fa34cfe1d 100644 --- a/pkg/parser/yaml/parser_test.go +++ b/pkg/parser/yaml/parser_test.go @@ -389,7 +389,7 @@ func Test_Resolve(t *testing.T) { ` parser := &Parser{} - resolved, err := parser.Resolve([]byte(have), "test.yaml", true) + resolved, err := parser.Resolve([]byte(have), "test.yaml", true, 15) require.NoError(t, err) require.Equal(t, []byte(have), resolved) } diff --git a/pkg/remediation/remediation.go b/pkg/remediation/remediation.go index 024e15ba589..6d621c12e7f 100644 --- a/pkg/remediation/remediation.go +++ b/pkg/remediation/remediation.go @@ -51,7 +51,7 @@ type Set struct { } // RemediateFile remediationSets the replacements first and secondly, the additions sorted down -func (s *Summary) RemediateFile(filePath string, remediationSet Set, openAPIResolveReferences bool) error { +func (s *Summary) RemediateFile(filePath string, remediationSet Set, openAPIResolveReferences bool, maxResolverDepth int) error { filepath.Clean(filePath) content, err := os.ReadFile(filePath) @@ -67,7 +67,7 @@ func (s *Summary) RemediateFile(filePath string, remediationSet Set, openAPIReso for i := range remediationSet.Replacement { r := remediationSet.Replacement[i] remediatedLines := replacement(&r, lines) - if len(remediatedLines) > 0 && willRemediate(remediatedLines, filePath, &r, openAPIResolveReferences) { + if len(remediatedLines) > 0 && willRemediate(remediatedLines, filePath, &r, openAPIResolveReferences, maxResolverDepth) { lines = s.writeRemediation(remediatedLines, lines, filePath, r.SimilarityID) } } @@ -83,7 +83,7 @@ func (s *Summary) RemediateFile(filePath string, remediationSet Set, openAPIReso for i := range remediationSet.Addition { a := remediationSet.Addition[i] remediatedLines := addition(&a, &lines) - if len(remediatedLines) > 0 && willRemediate(remediatedLines, filePath, &a, openAPIResolveReferences) { + if len(remediatedLines) > 0 && willRemediate(remediatedLines, filePath, &a, openAPIResolveReferences, maxResolverDepth) { lines = s.writeRemediation(remediatedLines, lines, filePath, a.SimilarityID) } } diff --git a/pkg/remediation/remediation_test.go b/pkg/remediation/remediation_test.go index e9826b2e2fc..b52be2e90c2 100644 --- a/pkg/remediation/remediation_test.go +++ b/pkg/remediation/remediation_test.go @@ -111,7 +111,7 @@ func Test_RemediateFile(t *testing.T) { tmpFileName := filepath.Join(os.TempDir(), "temporary-remediation"+utils.NextRandom()+filepath.Ext(filePathCopyFrom)) tmpFile := CreateTempFile(filePathCopyFrom, tmpFileName) - s.RemediateFile(tmpFile, tt.args.remediate, false) + s.RemediateFile(tmpFile, tt.args.remediate, false, 15) os.Remove(tmpFile) require.Equal(t, s.ActualRemediationDoneNumber, tt.actualRemediationDoneNumber) diff --git a/pkg/remediation/scan.go b/pkg/remediation/scan.go index 1a12e9eab13..7766bbaf413 100644 --- a/pkg/remediation/scan.go +++ b/pkg/remediation/scan.go @@ -37,9 +37,9 @@ type runQueryInfo struct { } // scanTmpFile scans a temporary file against a specific query -func scanTmpFile(tmpFile, queryID string, remediated []byte, openAPIResolveReferences bool) ([]model.Vulnerability, error) { +func scanTmpFile(tmpFile, queryID string, remediated []byte, openAPIResolveReferences bool, maxResolverDepth int) ([]model.Vulnerability, error) { // get payload - files, err := getPayload(tmpFile, remediated, openAPIResolveReferences) + files, err := getPayload(tmpFile, remediated, openAPIResolveReferences, maxResolverDepth) if err != nil { log.Err(err) @@ -82,7 +82,7 @@ func scanTmpFile(tmpFile, queryID string, remediated []byte, openAPIResolveRefer } // getPayload gets the payload of a file -func getPayload(filePath string, content []byte, openAPIResolveReferences bool) (model.FileMetadatas, error) { +func getPayload(filePath string, content []byte, openAPIResolveReferences bool, maxResolverDepth int) (model.FileMetadatas, error) { ext, _ := utils.GetExtension(filePath) var p []*parser.Parser var err error @@ -118,7 +118,7 @@ func getPayload(filePath string, content []byte, openAPIResolveReferences bool) } isMinified := minified.IsMinified(filePath, content) - documents, er := p[0].Parse(filePath, content, openAPIResolveReferences, isMinified) + documents, er := p[0].Parse(filePath, content, openAPIResolveReferences, isMinified, maxResolverDepth) 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 90c9f13bfad..69a558a65bc 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -49,7 +49,7 @@ func getBefore(line string) string { } // willRemediate verifies if the remediation actually removes the result -func willRemediate(remediated []string, originalFileName string, remediation *Remediation, openAPIResolveReferences bool) bool { +func willRemediate(remediated []string, originalFileName string, remediation *Remediation, openAPIResolveReferences bool, maxResolverDepth int) bool { filepath.Clean(originalFileName) // create temporary file tmpFile := filepath.Join(os.TempDir(), "temporary-remediation-"+utils.NextRandom()+"-"+filepath.Base(originalFileName)) @@ -75,7 +75,7 @@ func willRemediate(remediated []string, originalFileName string, remediation *Re } // scan the temporary file to verify if the remediation removed the result - results, err := scanTmpFile(tmpFile, remediation.QueryID, content, openAPIResolveReferences) + results, err := scanTmpFile(tmpFile, remediation.QueryID, content, openAPIResolveReferences, maxResolverDepth) if err != nil { log.Error().Msgf("failed to get results of query %s: %s", remediation.QueryID, err) diff --git a/pkg/resolver/file/file.go b/pkg/resolver/file/file.go index c2c3cbd24f1..38371df1d12 100644 --- a/pkg/resolver/file/file.go +++ b/pkg/resolver/file/file.go @@ -62,7 +62,7 @@ func isOpenAPI(fileContent []byte) bool { // Resolve - replace or modifies in-memory content before parsing func (r *Resolver) Resolve(fileContent []byte, path string, - resolveCount int, resolvedFilesCache map[string]ResolvedFile, + resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, resolveReferences bool) []byte { // handle panic during resolve process defer func() { @@ -77,7 +77,7 @@ func (r *Resolver) Resolve(fileContent []byte, path string, } if utils.Contains(filepath.Ext(path), []string{".yml", ".yaml"}) { - return r.yamlResolve(fileContent, path, resolveCount, resolvedFilesCache, resolveReferences) + return r.yamlResolve(fileContent, path, resolveCount, maxResolverDepth, resolvedFilesCache, resolveReferences) } var obj any err := r.unmarshler(fileContent, &obj) @@ -86,7 +86,7 @@ func (r *Resolver) Resolve(fileContent []byte, path string, } // resolve the paths - obj, _ = r.walk(fileContent, obj, obj, path, resolveCount, resolvedFilesCache, false, resolveReferences) + obj, _ = r.walk(fileContent, obj, obj, path, resolveCount, maxResolverDepth, resolvedFilesCache, false, resolveReferences) b, err := json.MarshalIndent(obj, "", "") if err == nil { @@ -101,7 +101,7 @@ func (r *Resolver) walk( fullObject interface{}, value any, path string, - resolveCount int, + resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, refBool bool, resolveReferences bool) (any, bool) { @@ -109,16 +109,16 @@ func (r *Resolver) walk( switch typedValue := value.(type) { case string: if filepath.Base(path) != typedValue { - return r.resolvePath(originalFileContent, fullObject, typedValue, path, resolveCount, resolvedFilesCache, refBool, resolveReferences) + return r.resolvePath(originalFileContent, fullObject, typedValue, path, resolveCount, maxResolverDepth, resolvedFilesCache, refBool, resolveReferences) } return value, false case []any: for i, v := range typedValue { - typedValue[i], _ = r.walk(originalFileContent, fullObject, v, path, resolveCount, resolvedFilesCache, refBool, resolveReferences) + typedValue[i], _ = r.walk(originalFileContent, fullObject, v, path, resolveCount, maxResolverDepth, resolvedFilesCache, refBool, resolveReferences) } return typedValue, false case map[string]any: - return r.handleMap(originalFileContent, fullObject, typedValue, path, resolveCount, resolvedFilesCache, resolveReferences) + return r.handleMap(originalFileContent, fullObject, typedValue, path, resolveCount, maxResolverDepth, resolvedFilesCache, resolveReferences) default: return value, false } @@ -129,13 +129,13 @@ func (r *Resolver) handleMap( fullObject interface{}, value map[string]interface{}, path string, - resolveCount int, + resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, resolveReferences bool, ) (any, bool) { for k, v := range value { isRef := strings.Contains(strings.ToLower(k), "$ref") - val, res := r.walk(originalFileContent, fullObject, v, path, resolveCount, resolvedFilesCache, isRef, resolveReferences) + val, res := r.walk(originalFileContent, fullObject, v, path, resolveCount, maxResolverDepth, resolvedFilesCache, isRef, resolveReferences) // check if it is a ref then add new details if valMap, ok := val.(map[string]interface{}); (ok || !res) && isRef { // Create RefMetadata and add it to the resolved value map @@ -156,7 +156,7 @@ func (r *Resolver) handleMap( } func (r *Resolver) yamlResolve(fileContent []byte, path string, - resolveCount int, resolvedFilesCache map[string]ResolvedFile, + resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, resolveReferences bool) []byte { var obj yaml.Node err := r.unmarshler(fileContent, &obj) @@ -167,7 +167,7 @@ func (r *Resolver) yamlResolve(fileContent []byte, path string, fullObjectCopy := obj // resolve the paths - obj, _ = r.yamlWalk(fileContent, &fullObjectCopy, &obj, path, resolveCount, resolvedFilesCache, false, resolveReferences) + obj, _ = r.yamlWalk(fileContent, &fullObjectCopy, &obj, path, resolveCount, maxResolverDepth, resolvedFilesCache, false, resolveReferences) if obj.Kind == 1 && len(obj.Content) == 1 { obj = *obj.Content[0] @@ -186,7 +186,7 @@ func (r *Resolver) yamlWalk( fullObject *yaml.Node, value *yaml.Node, path string, - resolveCount int, + resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, refBool bool, resolveReferences bool) (yaml.Node, bool) { @@ -196,7 +196,7 @@ func (r *Resolver) yamlWalk( if filepath.Base(path) != value.Value { return r.resolveYamlPath(originalFileContent, fullObject, value, path, - resolveCount, resolvedFilesCache, + resolveCount, maxResolverDepth, resolvedFilesCache, refBool, resolveReferences) } return *value, false @@ -208,7 +208,7 @@ func (r *Resolver) yamlWalk( } resolved, ok := r.yamlWalk(originalFileContent, fullObject, value.Content[i], path, - resolveCount, resolvedFilesCache, + resolveCount, maxResolverDepth, resolvedFilesCache, refBool, resolveReferences) if i >= 1 && refBool && (resolved.Kind == yaml.MappingNode || !ok) { @@ -255,7 +255,7 @@ func (r *Resolver) resolveYamlPath( fullObject *yaml.Node, v *yaml.Node, filePath string, - resolveCount int, + resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, refBool bool, resolveReferences bool) (yaml.Node, bool) { value := v.Value @@ -288,7 +288,7 @@ func (r *Resolver) resolveYamlPath( // Check if file has already been resolved, if not resolve it and save it for future references if _, ok := resolvedFilesCache[filename]; !ok { - if ret, isError := r.resolveFile(value, onlyFilePath, resolveCount, resolvedFilesCache, true, resolveReferences); isError { + if ret, isError := r.resolveFile(value, onlyFilePath, resolveCount, maxResolverDepth, resolvedFilesCache, true, resolveReferences); isError { if retYaml, yamlNode := ret.(yaml.Node); yamlNode { return retYaml, false } else { @@ -343,7 +343,7 @@ func (r *Resolver) returnResolveYamlPathValue( func (r *Resolver) resolveFile( value string, filePath string, - resolveCount int, + resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, yamlResolve bool, resolveReferences bool) (any, bool) { // open the file with the content to replace @@ -361,7 +361,7 @@ func (r *Resolver) resolveFile( // read the content fileContent, _ := io.ReadAll(file) - resolvedFile := r.Resolve(fileContent, filePath, resolveCount+1, resolvedFilesCache, resolveReferences) + resolvedFile := r.Resolve(fileContent, filePath, resolveCount+1, maxResolverDepth, resolvedFilesCache, resolveReferences) if yamlResolve { var obj yaml.Node @@ -402,7 +402,7 @@ func (r *Resolver) resolvePath( originalFileContent []byte, fullObject interface{}, value, filePath string, - resolveCount int, + resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, refBool bool, resolveReferences bool) (any, bool) { if resolveCount > constants.MaxResolvedFiles || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { @@ -430,7 +430,7 @@ func (r *Resolver) resolvePath( // Check if file has already been resolved, if not resolve it and save it for future references if _, ok := resolvedFilesCache[onlyFilePath]; !ok { - if ret, isError := r.resolveFile(value, onlyFilePath, resolveCount, resolvedFilesCache, false, resolveReferences); isError { + if ret, isError := r.resolveFile(value, onlyFilePath, resolveCount, maxResolverDepth, resolvedFilesCache, false, resolveReferences); isError { return ret, false } } diff --git a/pkg/resolver/file/file_test.go b/pkg/resolver/file/file_test.go index f80fcc10b02..572a8dd0a5f 100644 --- a/pkg/resolver/file/file_test.go +++ b/pkg/resolver/file/file_test.go @@ -3,8 +3,6 @@ package file import ( "encoding/json" "fmt" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "io/ioutil" "os" "path/filepath" @@ -12,6 +10,9 @@ import ( "strings" "testing" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "github.com/Checkmarx/kics/test" "gopkg.in/yaml.v3" ) @@ -82,7 +83,7 @@ func TestResolver_Resolve_With_ResolveReferences(t *testing.T) { t.Fatal(err) } - if got := r.Resolve(cont, tt.args.path, 0, make(map[string]ResolvedFile), true); !reflect.DeepEqual(prepareString(string(got)), prepareString(string(tt.want))) { + if got := r.Resolve(cont, tt.args.path, 0, 15, make(map[string]ResolvedFile), true); !reflect.DeepEqual(prepareString(string(got)), prepareString(string(tt.want))) { t.Errorf("Resolve() = %v, want = %v", prepareString(string(got)), prepareString(string(tt.want))) } }) @@ -144,7 +145,7 @@ func TestResolver_Resolve_Without_ResolveReferences(t *testing.T) { t.Fatal(err) } - if got := r.Resolve(cont, tt.args.path, 0, make(map[string]ResolvedFile), false); !reflect.DeepEqual(prepareString(string(got)), prepareString(string(tt.want))) { + if got := r.Resolve(cont, tt.args.path, 0, 15, make(map[string]ResolvedFile), false); !reflect.DeepEqual(prepareString(string(got)), prepareString(string(tt.want))) { t.Errorf("Resolve() = %v, want = %v", prepareString(string(got)), prepareString(string(tt.want))) } }) diff --git a/pkg/scan/client.go b/pkg/scan/client.go index f6685c55a6d..fef76485b5a 100644 --- a/pkg/scan/client.go +++ b/pkg/scan/client.go @@ -48,6 +48,7 @@ type Parameters struct { ParallelScanFlag int MaxFileSizeFlag int UseOldSeverities bool + MaxResolverDepth int } // Client represents a scan client diff --git a/pkg/scan/scan.go b/pkg/scan/scan.go index b56657df9ce..25f64b24eb2 100644 --- a/pkg/scan/scan.go +++ b/pkg/scan/scan.go @@ -137,7 +137,7 @@ func (c *Client) executeScan(ctx context.Context) (*Results, error) { return nil, nil } - if err = scanner.PrepareAndScan(ctx, c.ScanParams.ScanID, c.ScanParams.OpenAPIResolveReferences, *c.ProBarBuilder, + if err = scanner.PrepareAndScan(ctx, c.ScanParams.ScanID, c.ScanParams.OpenAPIResolveReferences, c.ScanParams.MaxResolverDepth, *c.ProBarBuilder, executeScanParameters.services); err != nil { log.Err(err) return nil, err diff --git a/pkg/scanner/scanner.go b/pkg/scanner/scanner.go index c74ada02e4f..673db8f9d44 100644 --- a/pkg/scanner/scanner.go +++ b/pkg/scanner/scanner.go @@ -16,6 +16,7 @@ func PrepareAndScan( ctx context.Context, scanID string, openAPIResolveReferences bool, + maxResolverDepth int, proBarBuilder progress.PbBuilder, services serviceSlice, ) error { @@ -27,7 +28,7 @@ func PrepareAndScan( for _, service := range services { wg.Add(1) - go service.PrepareSources(ctx, scanID, openAPIResolveReferences, &wg, errCh) + go service.PrepareSources(ctx, scanID, openAPIResolveReferences, maxResolverDepth, &wg, errCh) } go func() { diff --git a/test/main_test.go b/test/main_test.go index d153d1891df..c4ebe920613 100644 --- a/test/main_test.go +++ b/test/main_test.go @@ -171,7 +171,7 @@ func getFilesMetadatasWithContent(t testing.TB, filePath string, content []byte) files := make(model.FileMetadatas, 0) for _, parser := range combinedParser { - docs, err := parser.Parse(filePath, content, true, false) + docs, err := parser.Parse(filePath, content, true, false, 15) for _, document := range docs.Docs { require.NoError(t, err) files = append(files, model.FileMetadata{ diff --git a/test/queries_test.go b/test/queries_test.go index df65f1dcd94..d85b4231ffe 100644 --- a/test/queries_test.go +++ b/test/queries_test.go @@ -96,7 +96,7 @@ func testRemediationQuery(t testing.TB, entry queryEntry, vulnerabilities []mode for filePath := range temporaryRemediationSets { fix := temporaryRemediationSets[filePath].(remediation.Set) - err = summary.RemediateFile(filePath, fix, false) + err = summary.RemediateFile(filePath, fix, false, 15) os.Remove(filePath) if err != nil { require.NoError(t, err) From 7c1a42f7be37b9001a14c3e44cc400f05a433057 Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Fri, 3 May 2024 16:22:42 +0100 Subject: [PATCH 02/14] Remove MaxResolvedFiles constant --- internal/constants/constants.go | 3 --- pkg/resolver/file/file.go | 5 ++--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/internal/constants/constants.go b/internal/constants/constants.go index b72c9f94048..979eb620ca2 100644 --- a/internal/constants/constants.go +++ b/internal/constants/constants.go @@ -125,9 +125,6 @@ const ( // LogFormatPretty - print log more readable LogFormatPretty = "pretty" - - // MaxResolvedFiles - max files kics will resolve to prevent circular cycles - MaxResolvedFiles = 50 ) // GetRelease - returns the current release in the format 'kics@version' to be used by sentry diff --git a/pkg/resolver/file/file.go b/pkg/resolver/file/file.go index 38371df1d12..a340d3f5ee6 100644 --- a/pkg/resolver/file/file.go +++ b/pkg/resolver/file/file.go @@ -11,7 +11,6 @@ import ( "strconv" "strings" - "github.com/Checkmarx/kics/internal/constants" "github.com/Checkmarx/kics/pkg/analyzer" "gopkg.in/yaml.v3" @@ -259,7 +258,7 @@ func (r *Resolver) resolveYamlPath( resolvedFilesCache map[string]ResolvedFile, refBool bool, resolveReferences bool) (yaml.Node, bool) { value := v.Value - if resolveCount > constants.MaxResolvedFiles || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { + if resolveCount > maxResolverDepth || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { return *v, false } var splitPath []string @@ -405,7 +404,7 @@ func (r *Resolver) resolvePath( resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, refBool bool, resolveReferences bool) (any, bool) { - if resolveCount > constants.MaxResolvedFiles || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { + if resolveCount > maxResolverDepth || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { return value, false } var splitPath []string From 4fd608b115ee8ba96714e1570389f65f8c7a3d19 Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Fri, 3 May 2024 16:51:56 +0100 Subject: [PATCH 03/14] lint --- pkg/kics/resolver_sink.go | 10 ++++++++-- pkg/parser/parser.go | 6 +++++- pkg/remediation/scan.go | 6 +++++- pkg/resolver/file/file.go | 20 +++++++++++++++----- pkg/scan/scan.go | 4 +++- 5 files changed, 36 insertions(+), 10 deletions(-) diff --git a/pkg/kics/resolver_sink.go b/pkg/kics/resolver_sink.go index 54f48c8e4de..830c523d2c3 100644 --- a/pkg/kics/resolver_sink.go +++ b/pkg/kics/resolver_sink.go @@ -16,7 +16,11 @@ import ( "github.com/rs/zerolog/log" ) -func (s *Service) resolverSink(ctx context.Context, filename, scanID string, openAPIResolveReferences bool, maxResolverDepth int) ([]string, error) { +func (s *Service) resolverSink( + ctx context.Context, + filename, scanID string, + openAPIResolveReferences bool, + maxResolverDepth int) ([]string, error) { kind := s.Resolver.GetType(filename) if kind == model.KindCOMMON { return []string{}, nil @@ -41,7 +45,9 @@ func (s *Service) resolverSink(ctx context.Context, filename, scanID string, ope } if kind == model.KindHELM { - ignoreList, errorIL := s.getOriginalIgnoreLines(rfile.FileName, rfile.OriginalData, openAPIResolveReferences, isMinified, maxResolverDepth) + ignoreList, errorIL := s.getOriginalIgnoreLines( + rfile.FileName, rfile.OriginalData, + openAPIResolveReferences, isMinified, maxResolverDepth) if errorIL == nil { documents.IgnoreLines = ignoreList diff --git a/pkg/parser/parser.go b/pkg/parser/parser.go index b252759424e..8281e9d2780 100644 --- a/pkg/parser/parser.go +++ b/pkg/parser/parser.go @@ -123,7 +123,11 @@ func (c *Parser) CommentsCommands(filePath string, fileContent []byte) model.Com // Parse executes a parser on the fileContent and returns the file content as a Document, the file kind and // an error, if an error has occurred -func (c *Parser) Parse(filePath string, fileContent []byte, openAPIResolveReferences, isMinified bool, maxResolverDepth int) (ParsedDocument, error) { +func (c *Parser) Parse( + filePath string, + fileContent []byte, + openAPIResolveReferences, isMinified bool, + maxResolverDepth int) (ParsedDocument, error) { fileContent = utils.DecryptAnsibleVault(fileContent, os.Getenv("ANSIBLE_VAULT_PASSWORD_FILE")) if c.isValidExtension(filePath) { diff --git a/pkg/remediation/scan.go b/pkg/remediation/scan.go index 7766bbaf413..8efbe8e0a68 100644 --- a/pkg/remediation/scan.go +++ b/pkg/remediation/scan.go @@ -37,7 +37,11 @@ type runQueryInfo struct { } // scanTmpFile scans a temporary file against a specific query -func scanTmpFile(tmpFile, queryID string, remediated []byte, openAPIResolveReferences bool, maxResolverDepth int) ([]model.Vulnerability, error) { +func scanTmpFile( + tmpFile, queryID string, + remediated []byte, + openAPIResolveReferences bool, + maxResolverDepth int) ([]model.Vulnerability, error) { // get payload files, err := getPayload(tmpFile, remediated, openAPIResolveReferences, maxResolverDepth) diff --git a/pkg/resolver/file/file.go b/pkg/resolver/file/file.go index a340d3f5ee6..ff0bbd18492 100644 --- a/pkg/resolver/file/file.go +++ b/pkg/resolver/file/file.go @@ -108,16 +108,22 @@ func (r *Resolver) walk( switch typedValue := value.(type) { case string: if filepath.Base(path) != typedValue { - return r.resolvePath(originalFileContent, fullObject, typedValue, path, resolveCount, maxResolverDepth, resolvedFilesCache, refBool, resolveReferences) + return r.resolvePath( + originalFileContent, fullObject, typedValue, path, resolveCount, + maxResolverDepth, resolvedFilesCache, refBool, resolveReferences) } return value, false case []any: for i, v := range typedValue { - typedValue[i], _ = r.walk(originalFileContent, fullObject, v, path, resolveCount, maxResolverDepth, resolvedFilesCache, refBool, resolveReferences) + typedValue[i], _ = r.walk( + originalFileContent, fullObject, v, path, resolveCount, + maxResolverDepth, resolvedFilesCache, refBool, resolveReferences) } return typedValue, false case map[string]any: - return r.handleMap(originalFileContent, fullObject, typedValue, path, resolveCount, maxResolverDepth, resolvedFilesCache, resolveReferences) + return r.handleMap( + originalFileContent, fullObject, typedValue, path, resolveCount, + maxResolverDepth, resolvedFilesCache, resolveReferences) default: return value, false } @@ -287,7 +293,9 @@ func (r *Resolver) resolveYamlPath( // Check if file has already been resolved, if not resolve it and save it for future references if _, ok := resolvedFilesCache[filename]; !ok { - if ret, isError := r.resolveFile(value, onlyFilePath, resolveCount, maxResolverDepth, resolvedFilesCache, true, resolveReferences); isError { + if ret, isError := r.resolveFile( + value, onlyFilePath, resolveCount, maxResolverDepth, + resolvedFilesCache, true, resolveReferences); isError { if retYaml, yamlNode := ret.(yaml.Node); yamlNode { return retYaml, false } else { @@ -429,7 +437,9 @@ func (r *Resolver) resolvePath( // Check if file has already been resolved, if not resolve it and save it for future references if _, ok := resolvedFilesCache[onlyFilePath]; !ok { - if ret, isError := r.resolveFile(value, onlyFilePath, resolveCount, maxResolverDepth, resolvedFilesCache, false, resolveReferences); isError { + if ret, isError := r.resolveFile( + value, onlyFilePath, resolveCount, maxResolverDepth, + resolvedFilesCache, false, resolveReferences); isError { return ret, false } } diff --git a/pkg/scan/scan.go b/pkg/scan/scan.go index 25f64b24eb2..33743fa8084 100644 --- a/pkg/scan/scan.go +++ b/pkg/scan/scan.go @@ -137,7 +137,9 @@ func (c *Client) executeScan(ctx context.Context) (*Results, error) { return nil, nil } - if err = scanner.PrepareAndScan(ctx, c.ScanParams.ScanID, c.ScanParams.OpenAPIResolveReferences, c.ScanParams.MaxResolverDepth, *c.ProBarBuilder, + if err = scanner.PrepareAndScan( + ctx, + c.ScanParams.ScanID, c.ScanParams.OpenAPIResolveReferences, c.ScanParams.MaxResolverDepth, *c.ProBarBuilder, executeScanParameters.services); err != nil { log.Err(err) return nil, err From e139bdf094b3de0f42f63a7f6702de3b07956f49 Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Fri, 3 May 2024 17:38:59 +0100 Subject: [PATCH 04/14] Fix e2e and lint --- e2e/fixtures/assets/scan_help | 1 + internal/console/assets/scan-flags.json | 2 +- pkg/remediation/utils.go | 7 ++++++- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/e2e/fixtures/assets/scan_help b/e2e/fixtures/assets/scan_help index cfd526eee41..aaf3d2eb818 100644 --- a/e2e/fixtures/assets/scan_help +++ b/e2e/fixtures/assets/scan_help @@ -44,6 +44,7 @@ Flags: --input-data string path to query input data files -b, --libraries-path string path to directory with libraries (default "./assets/libraries") --max-file-size int max file size permitted for scanning, in MB (default 5) + --max-resolver-depth int max depth to which the resolver will traverse to resolve files (default 15) --minimal-ui simplified version of CLI output --no-progress hides the progress bar --old-severities uses old severities in query results diff --git a/internal/console/assets/scan-flags.json b/internal/console/assets/scan-flags.json index 2c4d0ff0500..ddc56476c9a 100644 --- a/internal/console/assets/scan-flags.json +++ b/internal/console/assets/scan-flags.json @@ -232,6 +232,6 @@ "flagType": "int", "shorthandFlag": "", "defaultValue": "15", - "usage": "maximum depth to which the resolver will traverse to resolve files" + "usage": "max depth to which the resolver will traverse to resolve files" } } diff --git a/pkg/remediation/utils.go b/pkg/remediation/utils.go index 69a558a65bc..23443a7efe1 100644 --- a/pkg/remediation/utils.go +++ b/pkg/remediation/utils.go @@ -49,7 +49,12 @@ func getBefore(line string) string { } // willRemediate verifies if the remediation actually removes the result -func willRemediate(remediated []string, originalFileName string, remediation *Remediation, openAPIResolveReferences bool, maxResolverDepth int) bool { +func willRemediate( + remediated []string, + originalFileName string, + remediation *Remediation, + openAPIResolveReferences bool, + maxResolverDepth int) bool { filepath.Clean(originalFileName) // create temporary file tmpFile := filepath.Join(os.TempDir(), "temporary-remediation-"+utils.NextRandom()+"-"+filepath.Base(originalFileName)) From 32ac2adc0597afc44ce46452843889702a1b61e5 Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Fri, 3 May 2024 17:59:00 +0100 Subject: [PATCH 05/14] Add feature to search in 'vars' folder when including ansible vars --- pkg/resolver/file/file.go | 43 +++++++++++++++++++++++++++++++-------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/pkg/resolver/file/file.go b/pkg/resolver/file/file.go index ff0bbd18492..8312a95742f 100644 --- a/pkg/resolver/file/file.go +++ b/pkg/resolver/file/file.go @@ -172,7 +172,7 @@ func (r *Resolver) yamlResolve(fileContent []byte, path string, fullObjectCopy := obj // resolve the paths - obj, _ = r.yamlWalk(fileContent, &fullObjectCopy, &obj, path, resolveCount, maxResolverDepth, resolvedFilesCache, false, resolveReferences) + obj, _ = r.yamlWalk(fileContent, &fullObjectCopy, &obj, path, resolveCount, maxResolverDepth, resolvedFilesCache, false, resolveReferences, false) if obj.Kind == 1 && len(obj.Content) == 1 { obj = *obj.Content[0] @@ -194,7 +194,8 @@ func (r *Resolver) yamlWalk( resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, refBool bool, - resolveReferences bool) (yaml.Node, bool) { + resolveReferences bool, + ansibleVars bool) (yaml.Node, bool) { // go over the value and replace paths with the real content switch value.Kind { case yaml.ScalarNode: @@ -202,7 +203,7 @@ func (r *Resolver) yamlWalk( return r.resolveYamlPath(originalFileContent, fullObject, value, path, resolveCount, maxResolverDepth, resolvedFilesCache, - refBool, resolveReferences) + refBool, resolveReferences, ansibleVars) } return *value, false default: @@ -210,11 +211,12 @@ func (r *Resolver) yamlWalk( for i := range value.Content { if i >= 1 { refBool = strings.Contains(value.Content[i-1].Value, "$ref") + ansibleVars = strings.Contains(value.Content[i-1].Value, "include_vars") } resolved, ok := r.yamlWalk(originalFileContent, fullObject, value.Content[i], path, resolveCount, maxResolverDepth, resolvedFilesCache, - refBool, resolveReferences) + refBool, resolveReferences, ansibleVars) if i >= 1 && refBool && (resolved.Kind == yaml.MappingNode || !ok) { // Create RefMetadata and add it to yaml Node @@ -262,7 +264,7 @@ func (r *Resolver) resolveYamlPath( filePath string, resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, - refBool bool, resolveReferences bool) (yaml.Node, bool) { + refBool bool, resolveReferences bool, ansibleVars bool) (yaml.Node, bool) { value := v.Value if resolveCount > maxResolverDepth || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { return *v, false @@ -279,16 +281,24 @@ func (r *Resolver) resolveYamlPath( value = checkServerlessFileReference(value) path := filepath.Join(filepath.Dir(filePath), value) - onlyFilePath := getPathFromString(path) - _, err := os.Stat(path) - if err != nil { - return *v, false + if ansibleVars { + exists, ansibleVarsPath := findAnsibleVarsPath(filepath.Dir(filePath), value) + if !exists { + return *v, false + } + path = ansibleVarsPath + } else { + _, err := os.Stat(path) + if err != nil { + return *v, false + } } if !contains(filepath.Ext(path), r.Extension) { return *v, false } + onlyFilePath := getPathFromString(path) filename := filepath.Clean(onlyFilePath) // Check if file has already been resolved, if not resolve it and save it for future references @@ -586,3 +596,18 @@ func checkServerlessFileReference(value string) string { } return value } + +func findAnsibleVarsPath(folder, filename string) (exists bool, ansibleVarsPath string) { + paths := []string{ + filepath.Join(folder, "vars", filename), + filepath.Join(folder, filename), + } + + for _, path := range paths { + if _, err := os.Stat(path); err == nil { + return true, path + } + } + + return false, "" +} From b248f62a89bf0189e9cfbbd8d203d1f41fcbca7e Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Fri, 3 May 2024 18:01:18 +0100 Subject: [PATCH 06/14] Small refactor --- pkg/resolver/file/file.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/pkg/resolver/file/file.go b/pkg/resolver/file/file.go index 8312a95742f..b4c9a55befa 100644 --- a/pkg/resolver/file/file.go +++ b/pkg/resolver/file/file.go @@ -102,8 +102,7 @@ func (r *Resolver) walk( path string, resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, - refBool bool, - resolveReferences bool) (any, bool) { + refBool, resolveReferences bool) (any, bool) { // go over the value and replace paths with the real content switch typedValue := value.(type) { case string: @@ -193,9 +192,7 @@ func (r *Resolver) yamlWalk( path string, resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, - refBool bool, - resolveReferences bool, - ansibleVars bool) (yaml.Node, bool) { + refBool, resolveReferences, ansibleVars bool) (yaml.Node, bool) { // go over the value and replace paths with the real content switch value.Kind { case yaml.ScalarNode: @@ -264,7 +261,7 @@ func (r *Resolver) resolveYamlPath( filePath string, resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, - refBool bool, resolveReferences bool, ansibleVars bool) (yaml.Node, bool) { + refBool, resolveReferences, ansibleVars bool) (yaml.Node, bool) { value := v.Value if resolveCount > maxResolverDepth || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { return *v, false @@ -362,7 +359,7 @@ func (r *Resolver) resolveFile( filePath string, resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, - yamlResolve bool, resolveReferences bool) (any, bool) { + yamlResolve, resolveReferences bool) (any, bool) { // open the file with the content to replace file, err := os.Open(filepath.Clean(filePath)) if err != nil { From 459d891f02cff62948cebd17a4a6eef0d848364e Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Fri, 3 May 2024 22:06:45 +0100 Subject: [PATCH 07/14] lint --- pkg/resolver/file/file.go | 60 +++++++++++++++++++++++---------------- 1 file changed, 35 insertions(+), 25 deletions(-) diff --git a/pkg/resolver/file/file.go b/pkg/resolver/file/file.go index b4c9a55befa..d6e1edc67ce 100644 --- a/pkg/resolver/file/file.go +++ b/pkg/resolver/file/file.go @@ -171,7 +171,9 @@ func (r *Resolver) yamlResolve(fileContent []byte, path string, fullObjectCopy := obj // resolve the paths - obj, _ = r.yamlWalk(fileContent, &fullObjectCopy, &obj, path, resolveCount, maxResolverDepth, resolvedFilesCache, false, resolveReferences, false) + obj, _ = r.yamlWalk( + fileContent, &fullObjectCopy, &obj, path, resolveCount, + maxResolverDepth, resolvedFilesCache, false, resolveReferences, false) if obj.Kind == 1 && len(obj.Content) == 1 { obj = *obj.Content[0] @@ -204,7 +206,8 @@ func (r *Resolver) yamlWalk( } return *value, false default: - refBool := false + refBool = false + ansibleVars = false for i := range value.Content { if i >= 1 { refBool = strings.Contains(value.Content[i-1].Value, "$ref") @@ -277,27 +280,11 @@ func (r *Resolver) resolveYamlPath( } else { // external file resolve value = checkServerlessFileReference(value) - path := filepath.Join(filepath.Dir(filePath), value) - if ansibleVars { - exists, ansibleVarsPath := findAnsibleVarsPath(filepath.Dir(filePath), value) - if !exists { - return *v, false - } - path = ansibleVarsPath - } else { - _, err := os.Stat(path) - if err != nil { - return *v, false - } - } - - if !contains(filepath.Ext(path), r.Extension) { + exists, path, onlyFilePath, filename := findFilePath(filepath.Dir(filePath), value, ansibleVars, r.Extension) + if !exists { return *v, false } - onlyFilePath := getPathFromString(path) - filename := filepath.Clean(onlyFilePath) - // Check if file has already been resolved, if not resolve it and save it for future references if _, ok := resolvedFilesCache[filename]; !ok { if ret, isError := r.resolveFile( @@ -594,13 +581,36 @@ func checkServerlessFileReference(value string) string { return value } -func findAnsibleVarsPath(folder, filename string) (exists bool, ansibleVarsPath string) { - paths := []string{ - filepath.Join(folder, "vars", filename), - filepath.Join(folder, filename), +func findFilePath( + folderPath, filename string, + ansibleVars bool, + extensions []string) (exists bool, path, onlyFilePath, cleanFilePath string) { + path = filepath.Join(folderPath, filename) + if ansibleVars { + if exists, ansibleVarsPath := findAnsibleVarsPath(folderPath, filename); !exists { + return false, "", "", "" + } else { + path = ansibleVarsPath + } + } else if _, err := os.Stat(path); err != nil { + return false, "", "", "" + } + + if !contains(filepath.Ext(path), extensions) { + return false, "", "", "" + } + + onlyFilePath = getPathFromString(path) + return true, path, onlyFilePath, filepath.Clean(onlyFilePath) +} + +func findAnsibleVarsPath(folderPath, filename string) (exists bool, ansibleVarsPath string) { + possiblePaths := []string{ + filepath.Join(folderPath, "vars", filename), + filepath.Join(folderPath, filename), } - for _, path := range paths { + for _, path := range possiblePaths { if _, err := os.Stat(path); err == nil { return true, path } From a4f3f72a6007e71b18c404ba2bd52f524899d1c9 Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Mon, 6 May 2024 14:13:12 +0100 Subject: [PATCH 08/14] Fix --- pkg/resolver/file/file.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pkg/resolver/file/file.go b/pkg/resolver/file/file.go index d6e1edc67ce..31ab13b4496 100644 --- a/pkg/resolver/file/file.go +++ b/pkg/resolver/file/file.go @@ -206,8 +206,8 @@ func (r *Resolver) yamlWalk( } return *value, false default: - refBool = false - ansibleVars = false + refBool := false + ansibleVars := false for i := range value.Content { if i >= 1 { refBool = strings.Contains(value.Content[i-1].Value, "$ref") @@ -266,7 +266,7 @@ func (r *Resolver) resolveYamlPath( resolvedFilesCache map[string]ResolvedFile, refBool, resolveReferences, ansibleVars bool) (yaml.Node, bool) { value := v.Value - if resolveCount > maxResolverDepth || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { + if resolveCount >= maxResolverDepth || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { return *v, false } var splitPath []string @@ -406,7 +406,7 @@ func (r *Resolver) resolvePath( resolveCount, maxResolverDepth int, resolvedFilesCache map[string]ResolvedFile, refBool bool, resolveReferences bool) (any, bool) { - if resolveCount > maxResolverDepth || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { + if resolveCount >= maxResolverDepth || (strings.HasPrefix(value, "#") && !refBool) || (value == "#" && refBool) { return value, false } var splitPath []string From aa773000bd52f5d906ac90da4c1bd0a731551957 Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Mon, 6 May 2024 14:13:51 +0100 Subject: [PATCH 09/14] Add unit tests --- pkg/resolver/file/file_test.go | 63 +++++++++++++++++++ .../main.yml | 6 ++ .../task.yml | 5 ++ .../vars/main.yml | 1 + .../main.yml | 6 ++ .../task.yml | 5 ++ .../variables.yml | 1 + 7 files changed, 87 insertions(+) create mode 100644 test/fixtures/resolve_ansible_vars_with_vars_folder/main.yml create mode 100644 test/fixtures/resolve_ansible_vars_with_vars_folder/task.yml create mode 100644 test/fixtures/resolve_ansible_vars_with_vars_folder/vars/main.yml create mode 100644 test/fixtures/resolve_ansible_vars_without_vars_folder/main.yml create mode 100644 test/fixtures/resolve_ansible_vars_without_vars_folder/task.yml create mode 100644 test/fixtures/resolve_ansible_vars_without_vars_folder/variables.yml diff --git a/pkg/resolver/file/file_test.go b/pkg/resolver/file/file_test.go index 572a8dd0a5f..9ed68ea10c9 100644 --- a/pkg/resolver/file/file_test.go +++ b/pkg/resolver/file/file_test.go @@ -152,6 +152,69 @@ func TestResolver_Resolve_Without_ResolveReferences(t *testing.T) { } } +func TestResolver_Resolve_Ansible_Vars(t *testing.T) { + err := test.ChangeCurrentDir("kics") + if err != nil { + t.Fatal(err) + } + type fields struct { + *Resolver + } + type args struct { + path string + } + tests := []struct { + name string + fields fields + args args + want []byte + }{ + { + name: "resolve ansible vars when vars folder is present", + fields: fields{ + Resolver: NewResolver(yaml.Unmarshal, yaml.Marshal, []string{".yml", ".yaml"}), + }, + args: args{ + path: filepath.ToSlash("test/fixtures/resolve_ansible_vars_with_vars_folder/main.yml"), + }, + want: []byte( + `-hosts:localhosttasks:-name:Includetask.ymlansible.builtin.include_tasks:-name:Addvariablesansible.builtin.include_vars:world:"World"-name:Printvariablefrommain.ymldebug:msg:"Hello{{world}}"-name:Includetask.ymlagainansible.builtin.include_tasks:-name:Addvariablesansible.builtin.include_vars:world:"World"-name:Printvariablefrommain.ymldebug:msg:"Hello{{world}}"`, + ), + }, + { + name: "resolve ansible vars when vars folder is not present", + fields: fields{ + Resolver: NewResolver(yaml.Unmarshal, yaml.Marshal, []string{".yml", ".yaml"}), + }, + args: args{ + path: filepath.ToSlash("test/fixtures/resolve_ansible_vars_without_vars_folder/main.yml"), + }, + want: []byte( + `-hosts:localhosttasks:-name:Includetask.ymlansible.builtin.include_tasks:-name:Addvariablesinclude_vars:world:"World"-name:Printvariablefrommain.ymldebug:msg:"Hello{{world}}"-name:Includetask.ymlagainansible.builtin.include_tasks:-name:Addvariablesinclude_vars:world:"World"-name:Printvariablefrommain.ymldebug:msg:"Hello{{world}}"`, + ), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + r := &Resolver{ + unmarshler: tt.fields.unmarshler, + marshler: tt.fields.marshler, + ResolvedFiles: tt.fields.ResolvedFiles, + Extension: tt.fields.Extension, + } + + cont, err := getFileContent(tt.args.path) + if err != nil { + t.Fatal(err) + } + + if got := r.Resolve(cont, tt.args.path, 0, 15, make(map[string]ResolvedFile), false); !reflect.DeepEqual(prepareString(string(got)), prepareString(string(tt.want))) { + t.Errorf("Resolve() = %v, want = %v", prepareString(string(got)), prepareString(string(tt.want))) + } + }) + } +} + func Test_IsOpenApi(t *testing.T) { err := test.ChangeCurrentDir("kics") if err != nil { diff --git a/test/fixtures/resolve_ansible_vars_with_vars_folder/main.yml b/test/fixtures/resolve_ansible_vars_with_vars_folder/main.yml new file mode 100644 index 00000000000..3243f3b13d9 --- /dev/null +++ b/test/fixtures/resolve_ansible_vars_with_vars_folder/main.yml @@ -0,0 +1,6 @@ +- hosts: localhost + tasks: + - name: Include task.yml + ansible.builtin.include_tasks: task.yml + - name: Include task.yml again + ansible.builtin.include_tasks: task.yml \ No newline at end of file diff --git a/test/fixtures/resolve_ansible_vars_with_vars_folder/task.yml b/test/fixtures/resolve_ansible_vars_with_vars_folder/task.yml new file mode 100644 index 00000000000..297b585cf21 --- /dev/null +++ b/test/fixtures/resolve_ansible_vars_with_vars_folder/task.yml @@ -0,0 +1,5 @@ +- name: Add variables + ansible.builtin.include_vars: main.yml +- name: Print variable from main.yml + debug: + msg: "Hello {{ world }}" \ No newline at end of file diff --git a/test/fixtures/resolve_ansible_vars_with_vars_folder/vars/main.yml b/test/fixtures/resolve_ansible_vars_with_vars_folder/vars/main.yml new file mode 100644 index 00000000000..e7727054bf3 --- /dev/null +++ b/test/fixtures/resolve_ansible_vars_with_vars_folder/vars/main.yml @@ -0,0 +1 @@ +world: "World" \ No newline at end of file diff --git a/test/fixtures/resolve_ansible_vars_without_vars_folder/main.yml b/test/fixtures/resolve_ansible_vars_without_vars_folder/main.yml new file mode 100644 index 00000000000..3243f3b13d9 --- /dev/null +++ b/test/fixtures/resolve_ansible_vars_without_vars_folder/main.yml @@ -0,0 +1,6 @@ +- hosts: localhost + tasks: + - name: Include task.yml + ansible.builtin.include_tasks: task.yml + - name: Include task.yml again + ansible.builtin.include_tasks: task.yml \ No newline at end of file diff --git a/test/fixtures/resolve_ansible_vars_without_vars_folder/task.yml b/test/fixtures/resolve_ansible_vars_without_vars_folder/task.yml new file mode 100644 index 00000000000..1985c8274ac --- /dev/null +++ b/test/fixtures/resolve_ansible_vars_without_vars_folder/task.yml @@ -0,0 +1,5 @@ +- name: Add variables + include_vars: variables.yml +- name: Print variable from main.yml + debug: + msg: "Hello {{ world }}" \ No newline at end of file diff --git a/test/fixtures/resolve_ansible_vars_without_vars_folder/variables.yml b/test/fixtures/resolve_ansible_vars_without_vars_folder/variables.yml new file mode 100644 index 00000000000..e7727054bf3 --- /dev/null +++ b/test/fixtures/resolve_ansible_vars_without_vars_folder/variables.yml @@ -0,0 +1 @@ +world: "World" \ No newline at end of file From e25159b2b3d488f8526a4892234ac2a33861ef5d Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Mon, 6 May 2024 16:04:26 +0100 Subject: [PATCH 10/14] Add e2e tests --- e2e/fixtures/E2E_CLI_094_RESULT.json | 54 +++++++++++++++++++ e2e/fixtures/E2E_CLI_095_RESULT.json | 29 ++++++++++ .../e2e-cli-094_max_resolver_depth_0.go | 30 +++++++++++ .../e2e-cli-095_max_resolver_depth_default.go | 29 ++++++++++ test/fixtures/resolve_circular_loop/main.yml | 6 +++ test/fixtures/resolve_circular_loop/task.yml | 5 ++ 6 files changed, 153 insertions(+) create mode 100644 e2e/fixtures/E2E_CLI_094_RESULT.json create mode 100644 e2e/fixtures/E2E_CLI_095_RESULT.json create mode 100644 e2e/testcases/e2e-cli-094_max_resolver_depth_0.go create mode 100644 e2e/testcases/e2e-cli-095_max_resolver_depth_default.go create mode 100644 test/fixtures/resolve_circular_loop/main.yml create mode 100644 test/fixtures/resolve_circular_loop/task.yml diff --git a/e2e/fixtures/E2E_CLI_094_RESULT.json b/e2e/fixtures/E2E_CLI_094_RESULT.json new file mode 100644 index 00000000000..4c93078eb14 --- /dev/null +++ b/e2e/fixtures/E2E_CLI_094_RESULT.json @@ -0,0 +1,54 @@ +{ + "kics_version": "development", + "files_scanned": 1, + "lines_scanned": 19, + "files_parsed": 1, + "lines_parsed": 19, + "lines_ignored": 0, + "files_failed_to_scan": 0, + "queries_total": 17, + "queries_failed_to_execute": 0, + "queries_failed_to_compute_similarity_id": 0, + "scan_id": "console", + "severity_counters": { + "CRITICAL": 0, + "HIGH": 0, + "INFO": 1, + "LOW": 0, + "MEDIUM": 0, + "TRACE": 0 + }, + "total_counter": 1, + "total_bom_resources": 0, + "start": "2024-05-06T15:45:28.1028682+01:00", + "end": "2024-05-06T15:45:29.6882643+01:00", + "paths": [ + "/path/test/fixtures/resolve_references" + ], + "queries": [ + { + "query_name": "Components Schema Definition Is Unused", + "query_id": "962fa01e-b791-4dcc-b04a-4a3e7389be5e", + "query_url": "https://swagger.io/specification/#components-object", + "severity": "INFO", + "platform": "OpenAPI", + "category": "Best Practices", + "experimental": false, + "description": "Components schemas definitions should be referenced or removed from Open API definition", + "description_id": "5cdc0f3b", + "files": [ + { + "file_name": "path\\test\\fixtures\\resolve_references\\swagger.yaml", + "similarity_id": "ff39e561509c13315ce34a0be602a974d63231b70cb5cdf778109e062302f8eb", + "line": 17, + "issue_type": "IncorrectValue", + "search_key": "components.schemas.{{MyResponse}}", + "search_line": -1, + "search_value": "", + "expected_value": "Schema should be used as reference somewhere", + "actual_value": "Schema is not used as reference" + } + ] + } + ] +} diff --git a/e2e/fixtures/E2E_CLI_095_RESULT.json b/e2e/fixtures/E2E_CLI_095_RESULT.json new file mode 100644 index 00000000000..3cd5bea8d6b --- /dev/null +++ b/e2e/fixtures/E2E_CLI_095_RESULT.json @@ -0,0 +1,29 @@ +{ + "kics_version": "development", + "files_scanned": 2, + "lines_scanned": 33, + "files_parsed": 2, + "lines_parsed": 6887, + "lines_ignored": 0, + "files_failed_to_scan": 0, + "queries_total": 0, + "queries_failed_to_execute": 0, + "queries_failed_to_compute_similarity_id": 0, + "scan_id": "console", + "severity_counters": { + "CRITICAL": 0, + "HIGH": 0, + "INFO": 0, + "LOW": 0, + "MEDIUM": 0, + "TRACE": 0 + }, + "total_counter": 0, + "total_bom_resources": 0, + "start": "2024-05-06T15:47:33.0217097+01:00", + "end": "2024-05-06T15:47:35.1422829+01:00", + "paths": [ + "/path/test/fixtures/resolve_circular_loop" + ], + "queries": [] +} diff --git a/e2e/testcases/e2e-cli-094_max_resolver_depth_0.go b/e2e/testcases/e2e-cli-094_max_resolver_depth_0.go new file mode 100644 index 00000000000..88ef2af39cb --- /dev/null +++ b/e2e/testcases/e2e-cli-094_max_resolver_depth_0.go @@ -0,0 +1,30 @@ +package testcases + +// E2E-CLI-094 - KICS scan and ignore references +// should perform the scan successfully and return exit code 20 +// this test is similar to E2E-CLI-071. Since the '--max-resolver-path' parameter is set to 0, it will not resolve any files +func init() { //nolint + testSample := TestCase{ + Name: "should perform a valid scan and not resolve references [E2E-CLI-094]", + Args: args{ + Args: []cmdArgs{ + []string{"scan", "-o", "/path/e2e/output", + "--output-name", "E2E_CLI_094_RESULT", + "-p", "\"/path/test/fixtures/resolve_references\"", + "-i", "6c35d2c6-09f2-4e5c-a094-e0e91327071d,962fa01e-b791-4dcc-b04a-4a3e7389be5e", + "--enable-openapi-refs", + "--max-resolver-depth", "0", + }, + }, + ExpectedResult: []ResultsValidation{ + { + ResultsFile: "E2E_CLI_094_RESULT", + ResultsFormats: []string{"json"}, + }, + }, + }, + WantStatus: []int{20}, + } + + Tests = append(Tests, testSample) +} diff --git a/e2e/testcases/e2e-cli-095_max_resolver_depth_default.go b/e2e/testcases/e2e-cli-095_max_resolver_depth_default.go new file mode 100644 index 00000000000..80a9686858c --- /dev/null +++ b/e2e/testcases/e2e-cli-095_max_resolver_depth_default.go @@ -0,0 +1,29 @@ +package testcases + +// E2E-CLI-095 - KICS scan and ignore references +// should perform the scan successfully and return exit code 0 +// this test sample contains a circular loop. It will stop after 15 iterations, having parsed 6887 lines +func init() { //nolint + testSample := TestCase{ + Name: "should perform a valid scan and resolve references [E2E-CLI-095]", + Args: args{ + Args: []cmdArgs{ + []string{"scan", "-o", "/path/e2e/output", + "--output-name", "E2E_CLI_095_RESULT", + "-p", "\"/path/test/fixtures/resolve_circular_loop\"", + "-i", "a88baa34-e2ad-44ea-ad6f-8cac87bc7c71", + "--max-resolver-depth", "15", + }, + }, + ExpectedResult: []ResultsValidation{ + { + ResultsFile: "E2E_CLI_095_RESULT", + ResultsFormats: []string{"json"}, + }, + }, + }, + WantStatus: []int{0}, + } + + Tests = append(Tests, testSample) +} diff --git a/test/fixtures/resolve_circular_loop/main.yml b/test/fixtures/resolve_circular_loop/main.yml new file mode 100644 index 00000000000..3243f3b13d9 --- /dev/null +++ b/test/fixtures/resolve_circular_loop/main.yml @@ -0,0 +1,6 @@ +- hosts: localhost + tasks: + - name: Include task.yml + ansible.builtin.include_tasks: task.yml + - name: Include task.yml again + ansible.builtin.include_tasks: task.yml \ No newline at end of file diff --git a/test/fixtures/resolve_circular_loop/task.yml b/test/fixtures/resolve_circular_loop/task.yml new file mode 100644 index 00000000000..6fbbfdaa7eb --- /dev/null +++ b/test/fixtures/resolve_circular_loop/task.yml @@ -0,0 +1,5 @@ +- name: Add variables + include_vars: main.yml +- name: Print variable from main.yml + debug: + msg: "Hello {{ world }}" \ No newline at end of file From ef39ba49fff625a0695aba9d48c670fec8e70bab Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Mon, 6 May 2024 16:55:36 +0100 Subject: [PATCH 11/14] Fix --- e2e/fixtures/E2E_CLI_095_RESULT.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/fixtures/E2E_CLI_095_RESULT.json b/e2e/fixtures/E2E_CLI_095_RESULT.json index 3cd5bea8d6b..ddd56ce055c 100644 --- a/e2e/fixtures/E2E_CLI_095_RESULT.json +++ b/e2e/fixtures/E2E_CLI_095_RESULT.json @@ -1,7 +1,7 @@ { "kics_version": "development", "files_scanned": 2, - "lines_scanned": 33, + "lines_scanned": 22, "files_parsed": 2, "lines_parsed": 6887, "lines_ignored": 0, From 92ff9ec56db7db9338b2936ea03aaa8310e12a2e Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Mon, 6 May 2024 18:05:55 +0100 Subject: [PATCH 12/14] Add comments --- pkg/resolver/file/file_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/pkg/resolver/file/file_test.go b/pkg/resolver/file/file_test.go index 9ed68ea10c9..cacde9bc262 100644 --- a/pkg/resolver/file/file_test.go +++ b/pkg/resolver/file/file_test.go @@ -152,6 +152,7 @@ func TestResolver_Resolve_Without_ResolveReferences(t *testing.T) { } } +// when using the key 'include_vars', the resolver will search for the file inside the folder 'current_path/vars' and then in the 'current_path'. func TestResolver_Resolve_Ansible_Vars(t *testing.T) { err := test.ChangeCurrentDir("kics") if err != nil { From d98a1ba8e4fa75ca1952899dc6938c379ccab8e8 Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Mon, 6 May 2024 19:27:56 +0100 Subject: [PATCH 13/14] Update docs --- docs/commands.md | 1 + docs/dockerhub.md | 2 ++ 2 files changed, 3 insertions(+) diff --git a/docs/commands.md b/docs/commands.md index 5bb01bc5836..f0394f9a83d 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -58,6 +58,7 @@ Use "kics [command] --help" for more information about a command. | --input-data string | path to query input data files| | -b, --libraries-path string | path to directory with libraries (default "./assets/libraries")| | --max-file-size int | max file size permitted for scanning, in MB (default 5)| +| --max-resolver-depth int | max depth to which the resolver will traverse to resolve files (default 15)| | --minimal-ui | simplified version of CLI output| | --no-progress | hides the progress bar| | --output-name string | name used on report creations (default "results")| diff --git a/docs/dockerhub.md b/docs/dockerhub.md index 93b95274791..6601d65462b 100644 --- a/docs/dockerhub.md +++ b/docs/dockerhub.md @@ -120,6 +120,8 @@ Flags: example: 'e69890e6-fce5-461d-98ad-cb98318dfc96,4728cd65-a20c-49da-8b31-9c08b423e4db' --input-data string path to query input data files -b, --libraries-path string path to directory with libraries (default "./assets/libraries") + --max-file-size int max file size permitted for scanning, in MB (default 5) + --max-resolver-depth int max depth to which the resolver will traverse to resolve files (default 15) --minimal-ui simplified version of CLI output --no-progress hides the progress bar --output-name string name used on report creations (default "results") From 95b766003f332219ed48381ae76129bbda4ae687 Mon Sep 17 00:00:00 2001 From: cx-ruiaraujo Date: Thu, 9 May 2024 18:40:42 +0100 Subject: [PATCH 14/14] Fix merge --- pkg/parser/buildah/parser_test.go | 2 +- pkg/resolver/file/file.go | 14 ++++++++------ pkg/resolver/file/file_test.go | 15 ++++++--------- 3 files changed, 15 insertions(+), 16 deletions(-) diff --git a/pkg/parser/buildah/parser_test.go b/pkg/parser/buildah/parser_test.go index 1f1f62f8cdd..9b474da8606 100644 --- a/pkg/parser/buildah/parser_test.go +++ b/pkg/parser/buildah/parser_test.go @@ -2,10 +2,10 @@ package buildah import ( "encoding/json" - "github.com/Checkmarx/kics/v2/pkg/model" "reflect" "testing" + "github.com/Checkmarx/kics/v2/pkg/model" "github.com/stretchr/testify/require" ) diff --git a/pkg/resolver/file/file.go b/pkg/resolver/file/file.go index 76249e7bf46..07d83e84762 100644 --- a/pkg/resolver/file/file.go +++ b/pkg/resolver/file/file.go @@ -2,17 +2,19 @@ package file import ( "encoding/json" + "errors" "fmt" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - "io/ioutil" + "io" "os" "path/filepath" - "reflect" + "regexp" + "strconv" "strings" - "testing" - "github.com/Checkmarx/kics/v2/test" + "github.com/Checkmarx/kics/v2/pkg/analyzer" + "github.com/Checkmarx/kics/v2/pkg/model" + "github.com/Checkmarx/kics/v2/pkg/utils" + "github.com/rs/zerolog/log" "gopkg.in/yaml.v3" ) diff --git a/pkg/resolver/file/file_test.go b/pkg/resolver/file/file_test.go index ff763312d98..cf45ebfe5bf 100644 --- a/pkg/resolver/file/file_test.go +++ b/pkg/resolver/file/file_test.go @@ -2,21 +2,18 @@ package file import ( "encoding/json" - "errors" "fmt" - "io" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "io/ioutil" "os" "path/filepath" - "regexp" - "strconv" + "reflect" "strings" + "testing" - "github.com/Checkmarx/kics/v2/pkg/analyzer" + "github.com/Checkmarx/kics/v2/test" "gopkg.in/yaml.v3" - - "github.com/Checkmarx/kics/v2/pkg/model" - "github.com/Checkmarx/kics/v2/pkg/utils" - "github.com/rs/zerolog/log" ) func TestResolver_Resolve_With_ResolveReferences(t *testing.T) {