Fix bugs with tracker counters #2735 (#2767)

Signed-off-by: João Reigota <[email protected]>

pkg/engine/provider/filesystem.go

@@ -26,30 +26,39 @@ var ErrNotSupportedFile = errors.New("invalid file format")
 func NewFileSystemSourceProvider(path string, excludes []string) (*FileSystemSourceProvider, error) {
 	log.Debug().Msgf("provider.NewFileSystemSourceProvider()")
 	ex := make(map[string][]os.FileInfo, len(excludes))
+	fs := &FileSystemSourceProvider{
+		path:     filepath.FromSlash(path),
+		excludes: ex,
+	}
 	for _, exclude := range excludes {
 		excludePaths, err := getExcludePaths(exclude)
 		if err != nil {
 			return nil, err
 		}
-		for _, excludePath := range excludePaths {
-			info, err := os.Stat(excludePath)
-			if err != nil {
-				if os.IsNotExist(err) {
-					continue
-				}
-				return nil, errors.Wrap(err, "failed to open excluded file")
-			}
-			if _, ok := ex[info.Name()]; !ok {
-				ex[info.Name()] = make([]os.FileInfo, 0)
-			}
-			ex[info.Name()] = append(ex[info.Name()], info)
+		if err := fs.AddExcluded(excludePaths); err != nil {
+			return nil, err
 		}
 	}
 
-	return &FileSystemSourceProvider{
-		path:     filepath.FromSlash(path),
-		excludes: ex,
-	}, nil
+	return fs, nil
+}
+
+// AddExcluded add new excluded files to the File System Source Provider
+func (s *FileSystemSourceProvider) AddExcluded(excludePaths []string) error {
+	for _, excludePath := range excludePaths {
+		info, err := os.Stat(excludePath)
+		if err != nil {
+			if os.IsNotExist(err) {
+				continue
+			}
+			return errors.Wrap(err, "failed to open excluded file")
+		}
+		if _, ok := s.excludes[info.Name()]; !ok {
+			s.excludes[info.Name()] = make([]os.FileInfo, 0)
+		}
+		s.excludes[info.Name()] = append(s.excludes[info.Name()], info)
+	}
+	return nil
 }
 
 func getExcludePaths(pathExpressions string) ([]string, error) {
@@ -71,6 +80,7 @@ func (s *FileSystemSourceProvider) GetBasePath() string {
 // GetSources tries to open file or directory and execute sink function on it
 func (s *FileSystemSourceProvider) GetSources(ctx context.Context,
 	extensions model.Extensions, sink Sink, resolverSink ResolverSink) error {
+	resolved := false
 	fileInfo, err := os.Stat(s.path)
 	if err != nil {
 		return errors.Wrap(err, "failed to open path")
@@ -94,21 +104,26 @@ func (s *FileSystemSourceProvider) GetSources(ctx context.Context,
 			return err
 		}
 
-		if shouldSkip, skipFolder := s.checkConditions(info, extensions, path); shouldSkip {
+		if shouldSkip, skipFolder := s.checkConditions(info, extensions, path, resolved); shouldSkip {
 			return skipFolder
 		}
 
 		// ------------------ Helm resolver --------------------------------
 		if info.IsDir() {
-			err = resolverSink(ctx, strings.ReplaceAll(path, "\\", "/"))
-			if err != nil {
-				sentry.CaptureException(err)
-				log.Err(err).
+			excluded, errRes := resolverSink(ctx, strings.ReplaceAll(path, "\\", "/"))
+			if errRes != nil {
+				sentry.CaptureException(errRes)
+				log.Err(errRes).
 					Msgf("Filesystem files provider couldn't Resolve Directory, file=%s", info.Name())
+				return nil
 			}
+			if errAdd := s.AddExcluded(excluded); errAdd != nil {
+				log.Err(errAdd).Msgf("Filesystem files provider couldn't exclude rendered Chart files, Chart=%s", info.Name())
+			}
+			resolved = true
 			return nil
 		}
-		// ------------------------------------------------------------
+		// -----------------------------------------------------------------
 
 		c, err := os.Open(filepath.Clean(path))
 		if err != nil {
@@ -136,14 +151,15 @@ func closeFile(file *os.File, info os.FileInfo) {
 	}
 }
 
-func (s *FileSystemSourceProvider) checkConditions(info os.FileInfo, extensions model.Extensions, path string) (bool, error) {
+func (s *FileSystemSourceProvider) checkConditions(info os.FileInfo, extensions model.Extensions,
+	path string, resolved bool) (bool, error) {
 	if info.IsDir() {
 		if f, ok := s.excludes[info.Name()]; ok && containsFile(f, info) {
 			log.Info().Msgf("Directory ignored: %s", path)
 			return true, filepath.SkipDir
 		}
 		_, err := os.Stat(filepath.Join(path, "Chart.yaml"))
-		if err != nil {
+		if err != nil || resolved {
 			return true, nil
 		}
 		return false, nil

pkg/engine/provider/filesystem_test.go

@@ -171,6 +171,22 @@ func TestFileSystemSourceProvider_GetSources(t *testing.T) { //nolint
 			},
 			wantErr: true,
 		},
+		{
+			name: "test_helm_source_provider",
+			fields: fields{
+				path:     "../../../test/fixtures/test_helm_subchart",
+				excludes: map[string][]os.FileInfo{},
+			},
+			args: args{
+				ctx: nil,
+				extensions: model.Extensions{
+					".dockerfile": dockerParser.Parser{},
+				},
+				sink:         mockSink,
+				resolverSink: mockResolverSink,
+			},
+			wantErr: false,
+		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
@@ -185,6 +201,41 @@ func TestFileSystemSourceProvider_GetSources(t *testing.T) { //nolint
 	}
 }
 
+func TestFileSystemSourceProvider_GetBasePath(t *testing.T) {
+	if err := test.ChangeCurrentDir("kics"); err != nil {
+		t.Errorf("failed to change dir: %s", err)
+	}
+	fsystem, err := initFs(filepath.FromSlash("test"), []string{})
+	if err != nil {
+		t.Errorf("failed to initialize a new File System Source Provider")
+	}
+	type feilds struct {
+		fs *FileSystemSourceProvider
+	}
+	tests := []struct {
+		name   string
+		feilds feilds
+		want   string
+	}{
+		{
+			name: "test_get_base_path",
+			feilds: feilds{
+				fs: fsystem,
+			},
+			want: "test",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.feilds.fs.GetBasePath()
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("GetBasePath() = %v, want = %v", got, tt.want)
+			}
+		})
+	}
+}
+
 // TestFileSystemSourceProvider_checkConditions tests the functions [checkConditions()] and all the methods called by them
 func TestFileSystemSourceProvider_checkConditions(t *testing.T) {
 	if err := test.ChangeCurrentDir("kics"); err != nil {
@@ -277,13 +328,66 @@ func TestFileSystemSourceProvider_checkConditions(t *testing.T) {
 				path:     tt.fields.path,
 				excludes: tt.fields.excludes,
 			}
-			if got, err := s.checkConditions(tt.args.info, tt.args.extensions, tt.args.path); got != tt.want.got || err != tt.want.err {
+			if got, err := s.checkConditions(tt.args.info, tt.args.extensions, tt.args.path, false); got != tt.want.got || err != tt.want.err {
 				t.Errorf("FileSystemSourceProvider.checkConditions() = %v, want %v", err, tt.want)
 			}
 		})
 	}
 }
 
+// TestFileSystemSourceProvider_AddExcluded tests the functions [AddExcluded()] and all the methods called by them
+func TestFileSystemSourceProvider_AddExcluded(t *testing.T) {
+	if err := test.ChangeCurrentDir("kics"); err != nil {
+		t.Errorf("failed to change dir: %s", err)
+	}
+	fsystem, err := initFs(filepath.FromSlash("test"), []string{})
+	if err != nil {
+		t.Errorf("failed to initialize a new File System Source Provider")
+	}
+	type feilds struct {
+		fs *FileSystemSourceProvider
+	}
+	type args struct {
+		excludePaths []string
+	}
+	tests := []struct {
+		name    string
+		feilds  feilds
+		args    args
+		want    []string
+		wantErr bool
+	}{
+		{
+			name: "test_add_excluded",
+			feilds: feilds{
+				fs: fsystem,
+			},
+			args: args{
+				excludePaths: []string{
+					"test/fixtures/config_test",
+				},
+			},
+			want: []string{
+				"config_test",
+			},
+			wantErr: false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := tt.feilds.fs.AddExcluded(tt.args.excludePaths)
+			if (err != nil) != tt.wantErr {
+				t.Errorf("AddExcluded() = %v, wantErr = %v", err, tt.wantErr)
+			}
+			got := getFSExcludes(tt.feilds.fs)
+			if !reflect.DeepEqual(got, tt.want) {
+				t.Errorf("AddExcluded() = %v, want = %v", got, tt.want)
+			}
+		})
+	}
+}
+
 var mockSink = func(ctx context.Context, filename string, content io.ReadCloser) error {
 	return nil
 }
@@ -292,16 +396,29 @@ var mockErrSink = func(ctx context.Context, filename string, content io.ReadClos
 	return errors.New("")
 }
 
-var mockResolverSink = func(ctx context.Context, filename string) error {
-	return nil
+var mockResolverSink = func(ctx context.Context, filename string) ([]string, error) {
+	return []string{}, nil
 }
 
-var mockErrResolverSink = func(ctx context.Context, filename string) error {
-	return errors.New("")
+var mockErrResolverSink = func(ctx context.Context, filename string) ([]string, error) {
+	return []string{}, errors.New("")
 }
 
 func checkStatErr(t *testing.T, err error) {
 	if err != nil {
 		t.Errorf("failed to get info: %s", err)
 	}
 }
+
+// initFs creates a new instance of File System Source Provider
+func initFs(path string, excluded []string) (*FileSystemSourceProvider, error) {
+	return NewFileSystemSourceProvider(path, excluded)
+}
+
+func getFSExcludes(fsystem *FileSystemSourceProvider) []string {
+	excluded := make([]string, 0)
+	for key := range fsystem.excludes {
+		excluded = append(excluded, key)
+	}
+	return excluded
+}

pkg/engine/provider/provider.go

@@ -3,7 +3,6 @@ package provider
 import (
 	"context"
 	"io"
-	"os"
 
 	"github.com/Checkmarx/kics/pkg/model"
 )
@@ -12,13 +11,12 @@ import (
 type Sink func(ctx context.Context, filename string, content io.ReadCloser) error
 
 // ResolverSink defines a sink function to be passed as reference to functions for resolved files/templates
-type ResolverSink func(ctx context.Context, filename string) error
+type ResolverSink func(ctx context.Context, filename string) ([]string, error)
 
 // SourceProvider is the interface that wraps the basic GetSources method.
 // GetBasePath returns base path of FileSystemSourceProvider
 // GetSources receives context, receive ID, extensions supported and a sink function to save sources
 type SourceProvider interface {
 	GetBasePath() string
 	GetSources(ctx context.Context, extensions model.Extensions, sink Sink, resolverSink ResolverSink) error
-	checkConditions(info os.FileInfo, extensions model.Extensions, path string) (bool, error)
 }

pkg/kics/resolver_sink.go

@@ -11,20 +11,24 @@ import (
 	"github.com/rs/zerolog/log"
 )
 
-func (s *Service) resolverSink(ctx context.Context, filename, scanID string) error {
-	s.Tracker.TrackFileFound()
+func (s *Service) resolverSink(ctx context.Context, filename, scanID string) ([]string, error) {
 	kind := s.Resolver.GetType(filename)
 	if kind == model.KindCOMMON {
-		return nil
+		return []string{}, nil
 	}
 	resFiles, err := s.Resolver.Resolve(filename, kind)
 	if err != nil {
-		return errors.Wrap(err, "failed to render file content")
+		return []string{}, errors.Wrap(err, "failed to render file content")
 	}
-	for _, rfile := range resFiles.File {
+
+	excluded := make([]string, len(resFiles.File))
+
+	for idx, rfile := range resFiles.File {
+		s.Tracker.TrackFileFound()
+		excluded[idx] = rfile.FileName
 		documents, _, err := s.Parser.Parse(rfile.FileName, rfile.Content)
 		if err != nil {
-			return errors.Wrap(err, "failed to parse file content")
+			return []string{}, errors.Wrap(err, "failed to parse file content")
 		}
 		for _, document := range documents {
 			_, err = json.Marshal(document)
@@ -47,6 +51,7 @@ func (s *Service) resolverSink(ctx context.Context, filename, scanID string) err
 			}
 			s.saveToFile(ctx, &file)
 		}
+		s.Tracker.TrackFileParse()
 	}
-	return nil
+	return excluded, nil
 }

pkg/kics/service.go

@@ -55,7 +55,7 @@ func (s *Service) StartScan(ctx context.Context, scanID string, hideProgress boo
 		func(ctx context.Context, filename string, rc io.ReadCloser) error {
 			return s.sink(ctx, filename, scanID, rc)
 		},
-		func(ctx context.Context, filename string) error { // Sink used for resolver files and templates
+		func(ctx context.Context, filename string) ([]string, error) { // Sink used for resolver files and templates
 			return s.resolverSink(ctx, filename, scanID)
 		},
 	); err != nil {
@@ -112,6 +112,5 @@ func (s *Service) saveToFile(ctx context.Context, file *model.FileMetadata) {
 	err := s.Storage.SaveFile(ctx, file)
 	if err == nil {
 		s.files = append(s.files, *file)
-		s.Tracker.TrackFileParse()
 	}
 }

pkg/kics/sink.go

@@ -46,6 +46,7 @@ func (s *Service) sink(ctx context.Context, filename, scanID string, rc io.Reade
 		}
 		s.saveToFile(ctx, &file)
 	}
+	s.Tracker.TrackFileParse()
 
 	return errors.Wrap(err, "failed to save file content")
 }