From 349eeb7bd547f907719f1f0741963f34edfc9a57 Mon Sep 17 00:00:00 2001 From: Simar Date: Wed, 1 May 2024 00:34:16 -0600 Subject: [PATCH 1/3] perf(misconf): Improve cause performance We only need to get the offending cause if the result is a failure. Signed-off-by: Simar --- pkg/misconf/scanner.go | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-) diff --git a/pkg/misconf/scanner.go b/pkg/misconf/scanner.go index 950ad73cbca6..e769bde1c52a 100644 --- a/pkg/misconf/scanner.go +++ b/pkg/misconf/scanner.go @@ -503,20 +503,26 @@ func NewCauseWithCode(underlying scan.Result) types.CauseMetadata { }, }) } - if code, err := underlying.GetCode(); err == nil { - cause.Code = types.Code{ - Lines: lo.Map(code.Lines, func(l scan.Line, i int) types.Line { - return types.Line{ - Number: l.Number, - Content: l.Content, - IsCause: l.IsCause, - Annotation: l.Annotation, - Truncated: l.Truncated, - Highlighted: l.Highlighted, - FirstCause: l.FirstCause, - LastCause: l.LastCause, - } - }), + + // only failures have a code cause + // failures can happen either due to lack of + // OR misconfiguration of something + if underlying.Status() == scan.StatusFailed { + if code, err := underlying.GetCode(); err == nil { + cause.Code = types.Code{ + Lines: lo.Map(code.Lines, func(l scan.Line, i int) types.Line { + return types.Line{ + Number: l.Number, + Content: l.Content, + IsCause: l.IsCause, + Annotation: l.Annotation, + Truncated: l.Truncated, + Highlighted: l.Highlighted, + FirstCause: l.FirstCause, + LastCause: l.LastCause, + } + }), + } } } return cause From 3483943523f54151fddf9d8e0de49f2e244591c5 Mon Sep 17 00:00:00 2001 From: Simar Date: Thu, 2 May 2024 00:39:46 -0600 Subject: [PATCH 2/3] use a bytes.NewReader instead --- pkg/iac/scan/code.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/pkg/iac/scan/code.go b/pkg/iac/scan/code.go index 5041b23ca2cc..e61b547988c6 100644 --- a/pkg/iac/scan/code.go +++ b/pkg/iac/scan/code.go @@ -1,6 +1,8 @@ package scan import ( + "bufio" + "bytes" "fmt" "io/fs" "path/filepath" @@ -149,7 +151,14 @@ func (r *Result) GetCode(opts ...CodeOption) (*Code, error) { Lines: nil, } - rawLines := strings.Split(string(content), "\n") + var rawLines []string + bs := bufio.NewScanner(bytes.NewReader(content)) + for bs.Scan() { + rawLines = append(rawLines, bs.Text()) + } + if bs.Err() != nil { + return nil, fmt.Errorf("failed to scan file : %w", err) + } var highlightedLines []string if settings.includeHighlighted { From 20047f8f545e832c6a7e44cf5137611392e327ff Mon Sep 17 00:00:00 2001 From: nikpivkin Date: Thu, 2 May 2024 15:55:44 +0600 Subject: [PATCH 3/3] perf: parse rego input once --- pkg/iac/rego/exceptions.go | 8 +++++--- pkg/iac/rego/scanner.go | 35 ++++++++++++++++++++++++++++------- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/pkg/iac/rego/exceptions.go b/pkg/iac/rego/exceptions.go index 7abe9e8a9afa..591c72abdbbb 100644 --- a/pkg/iac/rego/exceptions.go +++ b/pkg/iac/rego/exceptions.go @@ -3,9 +3,11 @@ package rego import ( "context" "fmt" + + "github.com/open-policy-agent/opa/ast" ) -func (s *Scanner) isIgnored(ctx context.Context, namespace, ruleName string, input interface{}) (bool, error) { +func (s *Scanner) isIgnored(ctx context.Context, namespace, ruleName string, input ast.Value) (bool, error) { if ignored, err := s.isNamespaceIgnored(ctx, namespace, input); err != nil { return false, err } else if ignored { @@ -14,7 +16,7 @@ func (s *Scanner) isIgnored(ctx context.Context, namespace, ruleName string, inp return s.isRuleIgnored(ctx, namespace, ruleName, input) } -func (s *Scanner) isNamespaceIgnored(ctx context.Context, namespace string, input interface{}) (bool, error) { +func (s *Scanner) isNamespaceIgnored(ctx context.Context, namespace string, input ast.Value) (bool, error) { exceptionQuery := fmt.Sprintf("data.namespace.exceptions.exception[_] == %q", namespace) result, _, err := s.runQuery(ctx, exceptionQuery, input, true) if err != nil { @@ -23,7 +25,7 @@ func (s *Scanner) isNamespaceIgnored(ctx context.Context, namespace string, inpu return result.Allowed(), nil } -func (s *Scanner) isRuleIgnored(ctx context.Context, namespace, ruleName string, input interface{}) (bool, error) { +func (s *Scanner) isRuleIgnored(ctx context.Context, namespace, ruleName string, input ast.Value) (bool, error) { exceptionQuery := fmt.Sprintf("endswith(%q, data.%s.exception[_][_])", ruleName, namespace) result, _, err := s.runQuery(ctx, exceptionQuery, input, true) if err != nil { diff --git a/pkg/iac/rego/scanner.go b/pkg/iac/rego/scanner.go index 6c1fc04c5065..8986f6eed54b 100644 --- a/pkg/iac/rego/scanner.go +++ b/pkg/iac/rego/scanner.go @@ -13,6 +13,7 @@ import ( "github.com/open-policy-agent/opa/ast" "github.com/open-policy-agent/opa/rego" "github.com/open-policy-agent/opa/storage" + "github.com/open-policy-agent/opa/util" "github.com/aquasecurity/trivy/pkg/iac/debug" "github.com/aquasecurity/trivy/pkg/iac/framework" @@ -161,7 +162,7 @@ func (s *Scanner) SetParentDebugLogger(l debug.Logger) { s.debug = l.Extend("rego") } -func (s *Scanner) runQuery(ctx context.Context, query string, input interface{}, disableTracing bool) (rego.ResultSet, []string, error) { +func (s *Scanner) runQuery(ctx context.Context, query string, input ast.Value, disableTracing bool) (rego.ResultSet, []string, error) { trace := (s.traceWriter != nil || s.tracePerResult) && !disableTracing @@ -180,7 +181,7 @@ func (s *Scanner) runQuery(ctx context.Context, query string, input interface{}, } if input != nil { - regoOptions = append(regoOptions, rego.Input(input)) + regoOptions = append(regoOptions, rego.ParsedInput(input)) } instance := rego.New(regoOptions...) @@ -342,6 +343,14 @@ func isPolicyApplicable(staticMetadata *StaticMetadata, inputs ...Input) bool { return false } +func parseRawInput(input any) (ast.Value, error) { + if err := util.RoundTrip(&input); err != nil { + return nil, err + } + + return ast.InterfaceToValue(input) +} + func (s *Scanner) applyRule(ctx context.Context, namespace, rule string, inputs []Input, combined bool) (scan.Results, error) { // handle combined evaluations if possible @@ -354,7 +363,12 @@ func (s *Scanner) applyRule(ctx context.Context, namespace, rule string, inputs qualified := fmt.Sprintf("data.%s.%s", namespace, rule) for _, input := range inputs { s.trace("INPUT", input) - if ignored, err := s.isIgnored(ctx, namespace, rule, input.Contents); err != nil { + parsedInput, err := parseRawInput(input.Contents) + if err != nil { + s.debug.Log("Error occurred while parsing input: %s", err) + continue + } + if ignored, err := s.isIgnored(ctx, namespace, rule, parsedInput); err != nil { return nil, err } else if ignored { var result regoResult @@ -364,7 +378,7 @@ func (s *Scanner) applyRule(ctx context.Context, namespace, rule string, inputs results.AddIgnored(result) continue } - set, traces, err := s.runQuery(ctx, qualified, input.Contents, false) + set, traces, err := s.runQuery(ctx, qualified, parsedInput, false) if err != nil { return nil, err } @@ -388,9 +402,15 @@ func (s *Scanner) applyRuleCombined(ctx context.Context, namespace, rule string, if len(inputs) == 0 { return nil, nil } + + parsed, err := parseRawInput(inputs) + if err != nil { + return nil, fmt.Errorf("failed to parse input: %w", err) + } + var results scan.Results - qualified := fmt.Sprintf("data.%s.%s", namespace, rule) - if ignored, err := s.isIgnored(ctx, namespace, rule, inputs); err != nil { + + if ignored, err := s.isIgnored(ctx, namespace, rule, parsed); err != nil { return nil, err } else if ignored { for _, input := range inputs { @@ -402,7 +422,8 @@ func (s *Scanner) applyRuleCombined(ctx context.Context, namespace, rule string, } return results, nil } - set, traces, err := s.runQuery(ctx, qualified, inputs, false) + qualified := fmt.Sprintf("data.%s.%s", namespace, rule) + set, traces, err := s.runQuery(ctx, qualified, parsed, false) if err != nil { return nil, err }