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

Possible Addition To Enhanced Reporting #6890

Open
ChrisHinchey opened this issue Nov 28, 2023 · 7 comments
Open

Possible Addition To Enhanced Reporting #6890

ChrisHinchey opened this issue Nov 28, 2023 · 7 comments

Comments

@ChrisHinchey
Copy link

Should getting an empty results array also be considered an error?

if results.any? { |r| !r[:exception].nil? && !r[:backtrace].nil? && r[:resource_class] != "noop" }

For a results array that doesn't have any data, the SAF team has determined that most of the time the control blocks are inside conditionals which is outside the style guide. We believe that this may be in the parameters of an error condition.

Provided below is an example:
shell_stig-1.json

@aaronlippold
Copy link
Collaborator

@Nik08 what do you think?

@Nik08
Copy link
Contributor

Nik08 commented Nov 29, 2023

I agree that from a point of style guide, it could be part of error.

But I also think this could be an add-on to the not reviewed condition - if the results are empty since that control has been skipped entirely.

@Nik08
Copy link
Contributor

Nik08 commented Nov 29, 2023

Just tried this out, if there are multiple controls, where some are within conditions and some not.

Then in that case, determining a conditionalised control (with condition returning false), from results is not possible. Since the data for that control is not appended in results. So handling such a conditionalised control (with condition returning false) in case of multiple controls does not seem very straightforward to me.

And the case when results are nil (In case if control file is empty or condition is false around control), we by default return passed status, which could be improved. Here base.rb#L126

@aaronlippold
Copy link
Collaborator

aaronlippold commented Nov 29, 2023

I think what I'm pointing out here is that the state should be defaulted to, at best, not reviewed, but in my opinion error, if the function that's supposed to determine the status of the control in any of our defined known states doesn't return with a result. In reality, we should never get to that else clause and if we do, it's because there's a problem.

As you pointed out in the line with in the base class.

If the control does not have any describe blocks it's usually because they are not evaluated due to the conditional. I'm not sure the default behavior should be to pass that control because we don't have any data one way or the other other if we are in a passing state. We are in an undetermined state and most likely in a state where we didn't properly capture the logic that would make sure we are always in a state of passing failing not reviewed or not applicable.

Most times this happens when the control is a loop and the appropriate only if or not applicable if condition was left out or in the case when you're iterating over some expected list, you're in the case when nothing was returned by that list and so you should be properly capturing whether that means the control ends up in a state of not reviewed or not applicable.

@aaronlippold
Copy link
Collaborator

Remember, in the compliance world it's "fail first ask questions later. " 🧐

@aaronlippold
Copy link
Collaborator

So I guess what we need to consider is if we're unable to determine the state of a control, it definitely shouldn't be passed. :-)

@aaronlippold
Copy link
Collaborator

aaronlippold commented Nov 29, 2023

So does that make sense to everyone if the requirement is not able to be bucketed into one of the four known states it defaults to an error state. And since we use metadata - backtrace waiver etc - or the test status data in the results array to determine that state when we don't have the metadata or the results we of course are in the unknown error state, and the only thing we do know is no results were produced by this control. And therefore that's all we can report error and there are no test results. Please review your control to ensure proper logic and function.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants