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

feat(misconf): add ability to disable checks by ID #7536

Merged
merged 3 commits into from
Sep 28, 2024

Conversation

nikpivkin
Copy link
Contributor

@nikpivkin nikpivkin commented Sep 18, 2024

Description

This PR adds the ability to disable Rego checks by their IDs as discussed here. This is not available to users, only within Trivy.

Related PRs

Checklist

  • I've read the guidelines for contributing to this repository.
  • I've followed the conventions in the PR title.
  • I've added tests that prove my fix is effective or that my feature works.
  • I've updated the documentation with the relevant information (if needed).
  • I've added usage information (if the PR introduces new options)
  • I've included a "before" and "after" example to the description (if the PR is a user interface change).

@nikpivkin
Copy link
Contributor Author

nikpivkin commented Sep 18, 2024

@simar7 Should we consider the namespace when ignoring if the user will have their own custom check with the same ID as our disabled check?

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>
@nikpivkin nikpivkin marked this pull request as ready for review September 19, 2024 14:23
@nikpivkin nikpivkin requested a review from simar7 as a code owner September 19, 2024 14:23
@simar7
Copy link
Member

simar7 commented Sep 19, 2024

@simar7 Should we consider the namespace when ignoring if the user will have their own custom check with the same ID as our disabled check?

I see your point but I don't think that's needed knowing they shouldn't be using an ID that a builtin check uses. Speaking of that:

  1. What happens if they use regardless? I would assume their check would run regardless, right?
  2. We should document that custom checks should use an ID that is not taken up by an existing check. Which we do here https://aquasecurity.github.io/trivy/v0.55/docs/scanner/misconfiguration/custom/#customavd_id-and-customid

@nikpivkin
Copy link
Contributor Author

Correct, id uniqueness is not checked.

@simar7
Copy link
Member

simar7 commented Sep 26, 2024

Correct, id uniqueness is not checked.

I added the check for namespace 74c9297

rego.WithDisabledCheckIDs(tt.disabledChecks...),
}

if tt.inputCheck != "" {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This condition will always be true. You can default to passing the option with a user-defined namespace, it will have no effect if there is no user check.

Copy link
Member

Choose a reason for hiding this comment

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

ah doh! thanks for the catch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed 7246de4

Signed-off-by: nikpivkin <nikita.pivkin@smartforce.io>
@simar7 simar7 enabled auto-merge September 28, 2024 06:31
@simar7 simar7 added this pull request to the merge queue Sep 28, 2024
Merged via the queue into aquasecurity:main with commit ef0a27d Sep 28, 2024
12 checks passed
@nikpivkin nikpivkin deleted the rego-disable-checks branch September 28, 2024 09:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants