Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ENGPROD-6507] filter secrets and licenses based on rego policy #4

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 40 additions & 10 deletions pkg/result/filter.go
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,14 @@

filteredVulns := filterVulnerabilities(result, severities, opt.IgnoreStatuses, ignoreConf.Vulnerabilities)
misconfSummary, filteredMisconfs := filterMisconfigurations(result, severities, opt.IncludeNonFailures, ignoreConf.Misconfigurations)
result.Secrets = filterSecrets(result, severities, ignoreConf.Secrets)
result.Licenses = filterLicenses(result.Licenses, severities, opt.IgnoreLicenses, ignoreConf.Licenses)
filteredSecrets := filterSecrets(result, severities, ignoreConf.Secrets)
filteredLicenses := filterLicenses(result.Licenses, severities, opt.IgnoreLicenses, ignoreConf.Licenses)

var ignoredMisconfs int
if opt.PolicyFile != "" {
var err error
var ignored int
filteredVulns, filteredMisconfs, ignored, err = applyPolicy(ctx, filteredVulns, filteredMisconfs, opt.PolicyFile)
filteredVulns, filteredMisconfs, ignored, filteredSecrets, filteredLicenses, err = applyPolicy(ctx, filteredVulns, filteredMisconfs, filteredSecrets, filteredLicenses, opt.PolicyFile)
if err != nil {
return xerrors.Errorf("failed to apply the policy: %w", err)
}
Expand All @@ -83,6 +83,8 @@
result.MisconfSummary.Exceptions += ignoredMisconfs
}
result.Misconfigurations = filteredMisconfs
result.Secrets = filteredSecrets
result.Licenses = filteredLicenses

return nil
}
Expand Down Expand Up @@ -221,11 +223,11 @@
}
}

func applyPolicy(ctx context.Context, vulns []types.DetectedVulnerability, misconfs []types.DetectedMisconfiguration,
policyFile string) ([]types.DetectedVulnerability, []types.DetectedMisconfiguration, int, error) {
func applyPolicy(ctx context.Context, vulns []types.DetectedVulnerability, misconfs []types.DetectedMisconfiguration, scrts []ftypes.SecretFinding, lics []types.DetectedLicense,

Check failure on line 226 in pkg/result/filter.go

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

tooManyResultsChecker: function has more than 5 results, consider to simplify the function (gocritic)

Check failure on line 226 in pkg/result/filter.go

View workflow job for this annotation

GitHub Actions / Test (ubuntu-latest)

tooManyResultsChecker: function has more than 5 results, consider to simplify the function (gocritic)
policyFile string) ([]types.DetectedVulnerability, []types.DetectedMisconfiguration, int, []ftypes.SecretFinding, []types.DetectedLicense, error) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you'll probably want to move these to before the int return. Or at least it makes the most logical sense to me that we should return the count of ignored things right before the error

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I retract this...it is not what I thought it was 😢

policy, err := os.ReadFile(policyFile)
if err != nil {
return nil, nil, 0, xerrors.Errorf("unable to read the policy file: %w", err)
return nil, nil, 0, nil, nil, xerrors.Errorf("unable to read the policy file: %w", err)
}

query, err := rego.New(
Expand All @@ -234,15 +236,15 @@
rego.Module("trivy.rego", string(policy)),
).PrepareForEval(ctx)
if err != nil {
return nil, nil, 0, xerrors.Errorf("unable to prepare for eval: %w", err)
return nil, nil, 0, nil, nil, xerrors.Errorf("unable to prepare for eval: %w", err)
}

// Vulnerabilities
var filteredVulns []types.DetectedVulnerability
for _, vuln := range vulns {
ignored, err := evaluate(ctx, query, vuln)
if err != nil {
return nil, nil, 0, err
return nil, nil, 0, nil, nil, err
}
if ignored {
continue
Expand All @@ -256,16 +258,44 @@
for _, misconf := range misconfs {
ignored, err := evaluate(ctx, query, misconf)
if err != nil {
return nil, nil, 0, err
return nil, nil, 0, nil, nil, err
}
if ignored {
ignoredMisconfs++
continue
}
filteredMisconfs = append(filteredMisconfs, misconf)
}
return filteredVulns, filteredMisconfs, ignoredMisconfs, nil

// Secrets
var filteredSecrets []ftypes.SecretFinding
for _, scrt := range scrts {
ignored, err := evaluate(ctx, query, scrt)
if err != nil {
return nil, nil, 0, nil, nil, err
}
if ignored {
continue
}
filteredSecrets = append(filteredSecrets, scrt)
}

// Licenses
var filteredLics []types.DetectedLicense
for _, lic := range lics {
ignored, err := evaluate(ctx, query, lic)
if err != nil {
return nil, nil, 0, nil, nil, err
}
if ignored {
continue
}
filteredLics = append(filteredLics, lic)
}

return filteredVulns, filteredMisconfs, ignoredMisconfs, filteredSecrets, filteredLics, nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So, maybe I'm wrong. It seems really weird that we're using a blind int to "only" return the count of ignored misconfigurations. Probably a follow-up change, but it seems like it would be useful to make a struct with counts of all the ignores

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@carsongee I just pushed a commit with a fix for the indentation issue. Should I open a follow-up ticket for the structure for ignores?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's start a "discussion" per their procedures and see if it is of interest for them

}

func evaluate(ctx context.Context, query rego.PreparedEvalQuery, input interface{}) (bool, error) {
results, err := query.Eval(ctx, rego.EvalInput(input))
if err != nil {
Expand Down
37 changes: 37 additions & 0 deletions pkg/result/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -722,6 +722,33 @@ func TestFilter(t *testing.T) {
Status: types.StatusFailure,
},
},
Secrets: []ftypes.SecretFinding{
{
RuleID: "generic-wanted-rule",
Severity: dbTypes.SeverityHigh.String(),
Title: "Secret that should pass filter on rule id",
StartLine: 1,
EndLine: 2,
Match: "*****",
},
{
RuleID: "generic-critical-rule",
Severity: dbTypes.SeverityHigh.String(),
Title: "Critical Secret shouldn't pass filter",
StartLine: 1,
EndLine: 2,
Match: "*****",
},
},
Licenses: []types.DetectedLicense{
{
Name: "GPL-3.0",
Severity: dbTypes.SeverityHigh.String(),
FilePath: "usr/share/gcc/python/libstdcxx/v6/printers.py",
Category: "restricted",
Confidence: 1,
},
},
},
},
},
Expand All @@ -746,6 +773,16 @@ func TestFilter(t *testing.T) {
Status: types.StatusFailure,
},
},
Secrets: []ftypes.SecretFinding{
{
RuleID: "generic-critical-rule",
Severity: dbTypes.SeverityHigh.String(),
Title: "Critical Secret shouldn't pass filter",
StartLine: 1,
EndLine: 2,
Match: "*****",
},
},
},
},
},
Expand Down
8 changes: 8 additions & 0 deletions pkg/result/testdata/test-ignore-policy-misconf.rego
Original file line number Diff line number Diff line change
Expand Up @@ -7,3 +7,11 @@ default ignore=false
ignore {
input.AVDID != "AVD-TEST-0001"
}

ignore {
input.RuleID == "generic-wanted-rule"
}

ignore {
input.Name == "GPL-3.0"
}
Loading