ljharb / repo-report

CLI to list all repos a user has access to, and report on their configuration in aggregate.
MIT License
24 stars 11 forks source link

Add metric: security policy #73

Closed ljharb closed 2 years ago

ljharb commented 2 years ago

This should respect a .github repo (org or user), as well as a repo-specific SECURITY.md.

abdumamdouh commented 2 years ago

i will work on this @ljharb

khattakdev commented 2 years ago

Hey @ljharb I went through the code with @abdumamdouh. I noticed that this metric is already added

SecurityPolicyEnabled: {
    extract: (item) => item.isSecurityPolicyEnabled,
    permissions: ["ADMIN", "MAINTAIN", "WRITE"],
  },

This is from config/metrics.js

However, the issue is it's returning the opposite of what it should return.

It returns false for the repo-report and repo-report-cache which have security police enabled. I tested it on my repositories and it returns false for those which have a security policy but returns true for the one that doesn't have any security policy.

Any hints/ideas on why this is happening?

image

dummy-1 has a security policy but it's printing false and true for other repos where I don't have a security policy.

ljharb commented 2 years ago

hmm. That strongly suggests that SecurityPolicyEnabled isn't what we think it is. There's a bunch of things in the API that are confusing like "disabled", and https://docs.github.com/en/rest/reference/repos doesn't reference isSecurityPolicyEnabled at all.

If you delete the security policy file from dummy-1, does it immediately switch back to true? If you then add it under khattakdev/.github, does it switch back to false? Maybe their boolean logic is just flipped, and we just need to add a !?

abdumamdouh commented 2 years ago

@ljharb Hello Jordan,

well, I tried to you know trace and try to reverse the boolean result of the "isSecurityPolicyEnabled" but the problem is that this is part of the query which come back as a response with many queries and then we use the response of the GraphQL as one that's why I can't get that specific boolean and reverse it using !

also, I tried to search if there is a way to query the boolean and reverse its result while querying using GraphQL but I didn't find a thing

so what do you recommend?

ljharb commented 2 years ago

I would expect we can reverse it in the extract function - there's a bunch of them doing !!, so they could likely do ! instead?

khattakdev commented 2 years ago

@ljharb tried what you said. I removed SECURITY.md from a repo and the API returns true.

So I guess we have to reverse it.

ljharb commented 2 years ago

Sounds good, let's do that then :-) let's please add a comment tho, and link it to https://github.com/github/feedback/discussions/13705 which i just filed?

ljharb commented 2 years ago

Closed in #75.