Skip to content

Commit

Permalink
Merge pull request #7043 from Checkmarx/ruiar/add-flag-max-resolver-d…
Browse files Browse the repository at this point in the history
…epth

feat(engine): add --max-resolver-depth flag
  • Loading branch information
cx-andrep authored May 13, 2024
2 parents 6a5eadf + 95b7660 commit cfbb06d
Show file tree
Hide file tree
Showing 52 changed files with 416 additions and 98 deletions.
1 change: 1 addition & 0 deletions docs/commands.md
Original file line number Diff line number Diff line change
Expand Up @@ -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")|
Expand Down
2 changes: 2 additions & 0 deletions docs/dockerhub.md
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
54 changes: 54 additions & 0 deletions e2e/fixtures/E2E_CLI_094_RESULT.json
Original file line number Diff line number Diff line change
@@ -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"
}
]
}
]
}
29 changes: 29 additions & 0 deletions e2e/fixtures/E2E_CLI_095_RESULT.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
{
"kics_version": "development",
"files_scanned": 2,
"lines_scanned": 22,
"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": []
}
1 change: 1 addition & 0 deletions e2e/fixtures/assets/scan_help
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
30 changes: 30 additions & 0 deletions e2e/testcases/e2e-cli-094_max_resolver_depth_0.go
Original file line number Diff line number Diff line change
@@ -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)
}
29 changes: 29 additions & 0 deletions e2e/testcases/e2e-cli-095_max_resolver_depth_default.go
Original file line number Diff line number Diff line change
@@ -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)
}
6 changes: 6 additions & 0 deletions internal/console/assets/scan-flags.json
Original file line number Diff line number Diff line change
Expand Up @@ -227,5 +227,11 @@
"shorthandFlag": "",
"defaultValue": "false",
"usage": "uses old severities in query results"
},
"max-resolver-depth": {
"flagType": "int",
"shorthandFlag": "",
"defaultValue": "15",
"usage": "max depth to which the resolver will traverse to resolve files"
}
}
1 change: 1 addition & 0 deletions internal/console/flags/scan_flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ const (
ParallelScanFile = "parallel"
MaxFileSizeFlag = "max-file-size"
UseOldSeveritiesFlag = "old-severities"
MaxResolverDepth = "max-resolver-depth"
)
1 change: 1 addition & 0 deletions internal/console/pre_scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
3 changes: 2 additions & 1 deletion internal/console/remediate.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions internal/console/scan.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions internal/constants/constants.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 12 additions & 5 deletions pkg/kics/resolver_sink.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,11 @@ 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
Expand All @@ -31,7 +35,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
Expand All @@ -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)
ignoreList, errorIL := s.getOriginalIgnoreLines(
rfile.FileName, rfile.OriginalData,
openAPIResolveReferences, isMinified, maxResolverDepth)
if errorIL == nil {
documents.IgnoreLines = ignoreList

Expand Down Expand Up @@ -100,11 +106,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
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/kics/resolver_sink_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
})
}
Expand Down
5 changes: 3 additions & 2 deletions pkg/kics/service.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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")
Expand Down
5 changes: 3 additions & 2 deletions pkg/kics/sink.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand All @@ -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
Expand Down
2 changes: 1 addition & 1 deletion pkg/parser/ansible/ini/config/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/parser/ansible/ini/hosts/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/parser/buildah/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions pkg/parser/buildah/parser_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand Down Expand Up @@ -320,7 +320,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
Expand Down
2 changes: 1 addition & 1 deletion pkg/parser/docker/parser.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
Loading

0 comments on commit cfbb06d

Please sign in to comment.