trivy icon indicating copy to clipboard operation
trivy copied to clipboard

Support Inline Filtering

Open Kurt-von-Laven opened this issue 1 year ago • 12 comments

Trivy already supports globally disabling a rule via .trivyignore, which is very valuable, but causes users to accept false negatives in order to eliminate false positives. To address this, most linters support disabling a specific rule in a specific file (or even on a specific line if applicable), making false positives far less likely.

Kurt-von-Laven avatar Sep 29 '22 19:09 Kurt-von-Laven

This issue is stale because it has been labeled with inactivity.

github-actions[bot] avatar Nov 29 '22 00:11 github-actions[bot]

I am interested in support for this feature as well.

jmreicha avatar Jan 09 '23 15:01 jmreicha

@Kurt-von-Laven @jmreicha Are you talking about misconfiguration as below?

resource "aws_security_group_rule" "my-rule" {
    type = "ingress"
    cidr_blocks = ["0.0.0.0/0"] #tfsec:ignore:aws-vpc-no-public-ingress-sgr
}

https://aquasecurity.github.io/tfsec/v0.61.3/getting-started/configuration/ignores/

knqyf263 avatar Jan 09 '23 15:01 knqyf263

Thank you for reaching out, but that link is to the documentation from an outdated release of tfsec, which is a Terraform linter. This issue is on the Trivy repository, which is a general purpose security scanner. For example, we can't find a way to disable AVD-DS-0002 but only for this file, so we are forced to disable the rule entirely.

Kurt-von-Laven avatar Jan 10 '23 01:01 Kurt-von-Laven

I just asked because Trivy has several capabilities like vulnerabilities, misconfigurations and secrets, and I was not sure which one you are talking about. I showed TFsec as an example as Trivy share the engine with TFsec. If you need this feature in Trivy as well, I believe we can make it easily.

knqyf263 avatar Jan 10 '23 09:01 knqyf263

Oh, gotcha; thanks for educating me. Yeah, I believe this feature would be helpful to many people on the basis that it seems to make its way into most popular linters eventually.

Kurt-von-Laven avatar Jan 10 '23 09:01 Kurt-von-Laven

@knqyf263 is the plan to support the same tfsec ignore capabilities across all misconfiguration scanners?

itaysk avatar Jan 10 '23 10:01 itaysk

We should have the same ignore capabilities for Terraform first. As for other languages, I think we can start with file level ignore like @Kurt-von-Laven showed.

knqyf263 avatar Jan 10 '23 10:01 knqyf263

if we're braking the implementation into multiple steps (first file, then line,block,args etc) then should we create a specific issue for allow ignoring check at the file level? or this issue will be closed when the PR is complete?

currently the ignore annotation starts with tfsec. I suppose we're going to change it to trivy? in this case, are we also changing it in terraform (which will be a breaking change)?

itaysk avatar Jan 10 '23 10:01 itaysk

adding a relevant proposal from openssf, not saying we should follow it now, just for future reference: https://docs.google.com/document/d/1811qanC8h9egv3Iszn_rrXGtAoSCz0YJGzp9vACjjH8

itaysk avatar Jan 10 '23 10:01 itaysk

if we're braking the implementation into multiple steps (first file, then line,block,args etc) then should we create a specific issue for allow ignoring check at the file level? or this issue will be closed when the PR is complete?

This issue could have large scope. File level, resource level, line level, etc. Also, it depends on configuration language. For example, JSON files cannot have comments. So, I'd hear the requirement from @Kurt-von-Laven. If @Kurt-von-Laven just needs file level ignore for Dockerfile, we will update this issue to narrow the scope.

currently the ignore annotation starts with tfsec. I suppose we're going to change it to trivy? in this case, are we also changing it in terraform (which will be a breaking change)?

I think we should keep both, tfsec:ignore:* and trivy:ignore:*. We mention only trivy:ignore in Trivy's doc.

knqyf263 avatar Jan 10 '23 11:01 knqyf263

Yes, our current needs will be met by support for file level ignore for Dockerfile alone, and I am all for incremental progress, so I agree with decomposing the issue.

Kurt-von-Laven avatar Jan 10 '23 21:01 Kurt-von-Laven

Interested in this as well, its the lack of inline ignores that is keeping us on tfsec and we may use another tool since they have support for similar functionality (wiz for example)

tinder-tder avatar Mar 03 '23 19:03 tinder-tder

@tinder-tder I think that tfsec:ignore already works through trivy terraform scanning as well (@simar7 @giorod3 please confirm?). we are currently working on generalizing it to work across trivy misconfiguration scanning as trivy:ignore.

itaysk avatar Mar 04 '23 09:03 itaysk

This is definitely a bit of a regression for us migrating from tfsec to trivy, now that tfsec is deprecated.

I have tried a mix of #trivy:ignore and #tfsec:ignore with a variety of vulnerability naming schemes (AVD-..., aws-ec2-....., uppercase, lowercase), and nothing seems to work to selectively filter findings inline while running Trivy.

jof avatar Mar 13 '23 19:03 jof

This is definitely a bit of a regression for us migrating from tfsec to trivy, now that tfsec is deprecated.

I have tried a mix of #trivy:ignore and #tfsec:ignore with a variety of vulnerability naming schemes (AVD-..., aws-ec2-....., uppercase, lowercase), and nothing seems to work to selectively filter findings inline while running Trivy.

Hi @jof can you tell me which version of trivy you are using. I can confirm that #tfsec:ignore: and #trivy:ignore: both work for inline ignores. trivy:ignore: was introduced in v0.38.0 and tfsec:ignore: was introduced in v0.24.0. Can you provide an example of how you were applying them.

Here is an example of how I verified it was working as expected:

Screenshot 2023-03-14 at 12 41 56 PM

this also works with tfsec:ignore:

giorod3 avatar Mar 14 '23 16:03 giorod3

@giorod3 -- thank you for a quality example. I was just testing before/after switching to Trivy and finding some used-to-be-ignored findings.

I am doing all my testing with Trivy 0.38.2

After some more testing, I found that this does work, but has a couple of small changes I needed to apply to my files:

  • Ignore comments must appear immediately before the resource; previously, we had free-form text comments between the ignore line and the resource
  • I needed to capitalize references to "AVD-..." strings

With these new findings, it seems like this functionality (inline ignore comments) does work, but users switching from tfsec may need some minor updates.

jof avatar Mar 14 '23 17:03 jof

I'm also running into some similar challenges with Dockerfiles in my repo. I'm noticing that the .trivyignore file needs to be adjacent to where trivy is being run. So, when scanning a repository, it seems limiting to only have repo-wide ignore options.

Since there are so many possible ways (or non-ways; e.g. JSON) to add inline comments, maybe a more flexible approach for trickier formats could be scoping the ignores by path inside of the config?

For example, I could fantasize about a .trivyignore.yaml file like:

---
ignores:
  - CVE-123
  - CVE-456
path_ignores:
  "some/sub/directory/Dockerfile":
    - AVD-DS-0002

Or something flat like:

*: CVE-123
*: CVE-456
some/sub/directory: AVD-DS-0002

I'm not sure how to nicely, uniquely refer to resources inside of config-scan files from a one-size-fits-all ignore config, though.

jof avatar Mar 14 '23 23:03 jof

that's a reasonable suggestion, but probably off-topic for this current issue. we can discuss this feature suggestion in a separate issue if needed

itaysk avatar Mar 15 '23 10:03 itaysk

Is this issue exclusively about misconfiguration? I'm looking for (an example of) how to selectively ignore CVEs when running trivy image --scanners vuln .... Similar to how Dependency Check allows granular control of the suppression. The .trivyignore file will ignore the CVE completely, but I want to only ignore it inside a specific JAR file.

If this is not the right issue, is there a more appropriate one yet?

ghost avatar Mar 22 '23 16:03 ghost

I am not affiliated with Trivy, but my understanding is that this issue has come to regard support for a special comment syntax to ignore Trivy rules in specific files (or on specific lines, but that has already been added).

Kurt-von-Laven avatar Mar 22 '23 22:03 Kurt-von-Laven

Note: the top hit on Google goes to an old version of Trivy which makes it look like Trivy still can't do this, whereas if you update the version at the top of the page and then go to the filtering section it mentions #trivy:ignore:.

Here is a direct link:

https://aquasecurity.github.io/trivy/v0.41/docs/configuration/filtering/#by-inline-comments

HariSekhon avatar May 13 '23 01:05 HariSekhon

Thanks for the note @HariSekhon , I've opened https://github.com/aquasecurity/trivy/discussions/4352 to discuss this

itaysk avatar May 13 '23 18:05 itaysk

Related discussion: https://github.com/aquasecurity/trivy/discussions/4573 and a broader one here: https://github.com/aquasecurity/trivy/discussions/3620

As far as more granular ignoring is concerned, I think it might be easier to support that within the .trivyignore file as @jof suggested. This will have two benefits:

  1. Easier to write ignore files. Rather than fiddling around with the correct syntax to ignore within a terraform config file, ignores can be centrally defined.
  2. The parsing logic for more granular ignores can be shared amongst different scanners. I have explained this in more detail here https://github.com/aquasecurity/trivy/discussions/3620#discussioncomment-6156471

Thoughts?

simar7 avatar Jun 12 '23 21:06 simar7

Filtering out by inline-comments for vulnerabilities (not just misconfigurations: https://aquasecurity.github.io/trivy/v0.44/docs/configuration/filtering/#by-inline-comments) would be a very helpful feature. Whether that's in the file itself (preferred) or in the .trivyignore file, it would be nice.

My specific use-case is a helm chart, no terraform like others above are using.

Shane-Braswell avatar Aug 08 '23 18:08 Shane-Braswell

ignores can be centrally defined

@simar7 would that go into the .trivyignore or the trivy.yaml file? I assume the .trivyignore?

AnaisUrlichs avatar Aug 09 '23 08:08 AnaisUrlichs

Having hit this a couple of times last week while migrating from tfsec, there are two things which would have helped save me a couple of CI cycles:

  1. If the documentation were updated to discuss this issue in an easily-discovered form - right now you're fighting years of stale information on Google / Stack Exchange / etc. and there isn't a top-level ignore topic in the documentation search. One thing which might be useful would be expanding the paragraph at the top of https://aquasecurity.github.io/trivy/v0.46/docs/configuration/filtering/ to have some of the common search keywords like “ignore”, “suppress”, or “false-positive”.
  2. There is a low-context search hit on .trivyignore.yaml which is almost what you want except that it doesn't have much context or intro (this would help when looking at the documentation search results) and it needs to lead with the warning that this isn't enabled by default and has to be explicitly enabled on the command-line. I think it'd make sense to enable it by default on the grounds that in the unlikely event someone has created a conflicting file for another purpose, they'd want to know about it sooner rather than later.
  3. The inline comment support is documented but it doesn't show up in the documentation search so it's easy to miss at the bottom of the page.
  4. The ignore check is case-sensitive. I think it should either be case-insensitive or report errors for anything it's told to ignore which doesn't match a known rule. This is especially easy to hit because the output doesn't include the ID, but it does include a URL which looks like it has the ID except that it's transformed to lower-case and won't have any effect in an ignore block unless you know to restore the expected case.
  5. Related to the previous point, it would also be useful if Trivy threw an error for any targeted ignore which didn't match an actual warning since that might indicate that you made a typo or a rule was renamed without a backwards mapping.

acdha avatar Oct 16 '23 18:10 acdha

thanks for the excellent feedback @acdha . @simar7 @AnaisUrlichs can you please review and create appropriate issues?

itaysk avatar Oct 17 '23 11:10 itaysk

thanks @acdha - I've created a couple of issues (docs and experience related) to improve this. Appreciate the feedback.

simar7 avatar Oct 17 '23 23:10 simar7

I feel we have enough to take away from this issue and have opened follow ups (https://github.com/aquasecurity/trivy/issues/5394 and https://github.com/aquasecurity/trivy/issues/5395) to specifically track improvements that we can close this. Please open a discussion if I've missed to account for anything.

simar7 avatar Oct 17 '23 23:10 simar7