Ability to "hush" individual warnings

Hmmn, my initial thought is that a "hush" feature would only be used by a few people and wouldn't warrant the additional code and UI controls. So, it means it's pretty low to almost no priority to me.

How do you envisions this feature working? What would it's controls look like? And then, why would you want a check to run but be "hushed"?

In this case, I might want to choose to "hush" the warning just for the @fyf view, but still have the check run on the site for other views.

I think a better solution is just to fix the offending view.

If this is a problem in other areas it could be considered, but it feels like a lot of code/UI for this feature. So marking as "works as designed" for now.

I'm going to re-open this issue because this is a problem we've been running into on multiple sites, where one of the checks fails, and every time we run the security review module, we need to re-investigate the issues we've already deemed are not actually problems. It would be fantastic if we were able to indicate a particular problem as "already reviewed", but not skip the check entirely so that we catch any new problems that crop up. Below are 3 examples of where we are running into this:

1. There are Views that do not provide any access checks. Webform default views do not have specific access restrictions set, but webform has its own access control and permissions that make these unnecessary. We'd prefer not to override the default webform views on all sites just to avoid the false positives. (Yes, there is also an issue for webform on this problem)
2. Dangerous tags were found in submitted content On most of our sites we have at least one node that had some crazy requirement where a JavaScript tag needed to be embedded. When we can, we try to budget in development time to put the tag into custom code, but when the tag changes frequently this is not a realistic solution. We only allow our administrators or developers access to the Full HTML format, so we know these people are trusted. We would like the ability to ignore this one node, but continue to check all the others for errant JavaScript.
3. Unsafe file extensions allowed in uploads. On several of our sites we allow (trusted) administrators to upload .html files. We provide this file type as an accessible alternative to a PDF. These specific files won't contain any dangerous HTML code, so we would like to ignore this particular field (it's a file upload field on a "HTML" media entity), but continue to check all other places where file uploads are permitted.

I'm not sure how much work it would be to allow individual issues to be ignored, but they implemented something similar recently for the editoria11y module. We have developer resources we can contribute to a solution if others feel it is something worth pursuing.

If the checks could be "hushed" in a way that would then make noise again if a different instance of the problem showed up then I think that would be a reasonable solution. What do you think?

All 3 of those items feel reasonable to me and I agree it would be nice to make them hushable.

For item 3 specifically, that feels like it should be an improvement to that upload test to confirm what permissions are required (if that's possible) so it doesn't even need a hush for that scenario.

I'm also not voicing this as an active maintainer of the module, so take what I said with a grain of salt ;)

> If the checks could be "hushed" in a way that would then make noise again if a different instance of the problem showed up then I think that would be a reasonable solution. What do you think?

Yes, that was what I was hoping for also.

> For item 3 specifically, that feels like it should be an improvement to that upload test to confirm what permissions are required (if that's possible) so it doesn't even need a hush for that scenario.

That would also be ideal.

I wonder if the same could be done for #2, we could also check the permissions on the text format that accompanies the problematic text (assuming it is filtered text).

If you have the resources/time then I would say please submit a patch for it, think it sounds like a neat idea that would only add value.
One feature that I think will be needed (just like editora11y does) is the ability to view all the "hushed" reviews. And ability to unhush (know that's not a word) reviews.

So in #2678112: Make fields check more useful with risky content (allowlist of content?) we are adding the ability to ignore specific violations of that test.

Adding a new array to display hushed findings

Could that be something we implement for other tests if needed?

Seems like a solid idea to me. Thanks!

Sweet so I'll open a meta so we can evaluate the other checks and see what other ones could benefit from a hushed ability.

While digging into #2678112: Make fields check more useful with risky content (allowlist of content?) it did make me wonder if this could be generic, but with every check testing something different I wasn't too sure.

Thank you so much for working on these improvements. I've requested time for devs to work on my 3rd request: Unsafe file extensions allowed in uploads. I didn't see a meta, so I created https://www.drupal.org/project/security_review/issues/3338061. I'll post our plan there when we start work on it :)

So need to carve out some time this week but going to open a meta where we will check/decide which checks should have hushed ability. May have some time tonight

The ticket you opened is valid but checking vendor directory probably not.

Opened #3338073: [Meta] - Hush Check where we can discuss what does and does not need hushed abilities.