search

dev-sec / cis-dil-benchmark

CIS Distribution Independent Linux Benchmark - InSpec Profile
Apache License 2.0
149 stars 92 forks source link

Fix lint #104

Closed schurzi closed 3 years ago

deric4 commented 3 years ago

From technical standpoint, everything looks good. I do have a question though, are Inspec Profiles expected to adhere to the default cops or should there be an attempt to align with the Inspec Docs/Style Guide:

Avoiding unnecessary parentheses in matchers: https://docs.chef.io/inspec/style/#avoid-unnecessary-parentheses-in-matchers

some examples here: https://github.com/dev-sec/cis-dil-benchmark/pull/90#issuecomment-727001731

schurzi commented 3 years ago

nice catch @deric4.

I already disabled Lint/AmbiguousBlockAssociation because of this. In the case here I would also need to disable Lint/AmbiguousRegexpLiteral. when this is done the autofix will no longer meddle with these parentheses.

I think the best approach is to get this in a state, where we can trust the autofix and end up with consistent code. The best(tm) approach should be to trust the linter completely, since every disabled rule leads to possible ambiguity and we need people to remember the special inspec rules.

@chris-rock what do you think?

chris-rock commented 3 years ago

I agree that it is confusing to have two different baselines. I would argue that Inspec policies are more geared towards security professionals, therefore it is okay to deviate. In any case we should make sure its the same for cross-policies for our baselines.

Personally, I would go with the simpler solution in this case, therefore I prefer it { should eq value }.

schurzi commented 3 years ago

thank you for clearing this up. The code is now aligned with that decision.