Improve policy language consistency

[derekmurawsky]

TL;DR

First off, fantastic tool. It's fast and I love to see something of this quality in the open source world.

However, I ran into an odd thing that could be a "bug", but is more likely a feature enhancement? Specifically, the policy title language used is not consistent and that leads to confusion when looking at the results in tabular form.

Examples:

• "Workflows Are Allowed To Approve Pull Requests", status failed. Ok, makes sense. The workflows are allowed to do this, that's bad, so we failed it. But when we make a change and pass it... are we passing because the workflows are allowed to approve the requests?
• But then you have: "Default branch is not protected", status passed. That doesn't make sense. The default branch is protected. Are you testing that it is or is not protected? I would think that a fail of "branch not protected" would mean the branch is in fact protected. The use of not is odd.
• "Vulnerability Alerts Is Not Enabled", status passed. Aside from the is/are issue, is this checking that the alerts are not enabled, and I passed that they are not enabled? Or is it saying that they should be enabled and I passed because they are enabled?

I would suggest changing the policy titles to be affirmative or negative, and then make the test results clearly align to them throughout all your policy titles. This would greatly aid in clarity.

Example:

• "Workflows should not be allowed to approve pull requests" pass/fail is clear in this case
• "Default branch should be protected" pass/fail is clear in this case and it is consistent with the previous
• "Vulnerability alerts should be enabled" pass/fail is clear and is consistent with previous

Also note, in my previous language there is a case to be made that "should" be changed to "must" if you want to conform with policy and standards language like RFC 2119 / 8174 / etc

Again, I love the idea of this tool and am really impressed with what you have done already. It's very useful. Just noticed this oddity and it made me a few other folks on my team go "huh?" so I thought I would share. Thanks again for a great tool, and I look forward to seeing where it goes. :)

Detailed design

No response

Additional information

Yes, this is pedantic. I do think it's important, though. I'm happy to help with suggested edits, but didn't want to do so unless there was appetite for it.

[derekmurawsky]

Additional thought, and possibly should be a separate issue... Should there be ID numbers of some kind for each of these policies? See examples like the CIS benchmarks. Here's an example from a docker benchmark scanner that clearly maps to those rules/policies. https://github.com/dev-sec/cis-docker-benchmark/blob/master/controls/container_images.rb

[noamd-legit]

Thanks for your kind feedback @derekmurawsky! 🔥
This is a very valid point and we'll appreciate your help.
Actually, we started a collaboration with "OSSF security best practices" working group to create a "SCM Best Practices" guide and we'll probably need to modify the policies as you suggest sometime soon.

Regarding the ID numbers, currently the policy names act as unique IDs, for what use case do you think unique ID numbers could be useful for?

[derekmurawsky]

I found you through the OSSF Security Best Practices group. I'm a lurker/random contributor over there. 😄 I have a few philosophical issues with some of the policies you suggest, but I agree with the ones I've seen so far from a purely security perspective. (See previously linked comment for examples).

RE: ID Numbers When referencing, it is much easier to reference a number, like GH-1, than a name. It also allows the policy name to change without breaking other references (see CIS benchmark example in previous comment). Like database primary/foreign keys; you want to reference the key so names and other things can change easily. When looking at most standards out there, like NIST, CIS, CSA, etc, they all reference each-other by specific numbers and not titles. See this link for one example of such a mapping. I would suggest doing the same here for both consistency and ease of reference.

RE: Re-Titling, I'll try to get a draft together this week.

[derekmurawsky]

I worked on this a bit today and had a sample ready to push. The contributing guide says to open a PR, but mentions nothing about forking. Is it expected that I fork and PR from there? I tried pushing a docs/retitle-for-clarity branch and it was denied. I just forked and PR'd from there as I would probably forget otherwise. 😆 Let me know if there is alternate preferred process to follow.

[noamd-legit]

Cool that you found us there 😄

RE: ID Number:: Good point. Reviewing your pull requests made it clear to me how important it is. I'll create an issue for adding policies id numbers.

[derekmurawsky]

Another general note: I believe it would make sense to separate the higher-level policy ideas from the specific implementations. You've started doing this by having groups of policies for specific areas, but I think it would be good to formalize that from a docs/policy side as well. I have no idea how complicated that would be to implement technically, but I think it would make life much easier in the long run, especially around the OSSF stuff. Perhaps this is a separate issue as well?

Examples in industry of what I call "The Policy Pyramid"

• https://frsecure.com/blog/differentiating-between-policies-standards-procedures-and-guidelines/
• https://www.complianceforge.com/governance-risk-compliance-grc/ (see the Governance section)

Examples:
Policy Section: Default Branch Controls
Specific Policy: The default branch should be protected
Implementation: The specific tests and implementation details for the various SCMs

Policy Section: Default Branch Controls
Specific Policy: The default branch should require review by more than two users before allowing a merge
Implementation: The specific tests and implementation details for the various SCMs

Policy Section: Global SCM Controls
Specific Policy: Limit number of Administrators
Implementation: The specific tests and implementation details for the various SCMs

By taking this approach, you can be more DRY as well. You only write the reasoning for the policy once at a higher level and then link to it for your specific implementations. This could also affect your numbering in #141 so I thought it best to bring up now.

There was a great example of this that I saw in one of the OPA-Related projects, but I can't find it off the cuff. I'll link it later if I can track it down.

ETA: Not the one I was originally thinking of, but regula does a decent job of mapping their controls using metadata/tags. This would be a huge value-add to enterprise users. This example specifically links to the AWS CIS Benchmark. If you developed a policy, you could link the same way. And if you support generic tagging, you could link to industry standards as well.

[derekmurawsky]

Just submitted the updates. Should be good to go now.