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

Support using legitify action with GITHUB_TOKEN #265

Open
noamd-legit opened this issue Nov 15, 2023 · 5 comments
Open

Support using legitify action with GITHUB_TOKEN #265

noamd-legit opened this issue Nov 15, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@noamd-legit
Copy link
Contributor

TL;DR

Currently, it is not possible to use the action with the automatically generated GITHUB_TOKEN because it has no permission to execute the following API:

https://api.github.com/user/orgs

It should be possible to use it when executing the action with the analyze_self option

Detailed design

No response

Additional information

No response

@mackowski
Copy link

Hey @noamd-legit is there a plan to implement this? I belive this is also a blocker to use legitify in a GitHub App.

@molson504x
Copy link
Contributor

@noamd-legit +1 for this - this is a blocker from a client being able to use this product because they aren't comfortable with using a PAT token in this instance.

@molson504x
Copy link
Contributor

@noamd-legit - I did a bunch of digging into Legitify's source code, and I think I see why this isn't an "easy fix" scenario, and instead would require a pretty large refactor. And despite what we originally thought, it's NOT because they have to call out to the /user/orgs API endpoint. This is going to be a long post, but tl;dr it's way more than just one API call that has a dependency, and this is not an easy fix.

That all being said, I can't help but feel like this effort still needs to be done since GH PAT tokens are tied to a specific user, and in this case the user also has to have quite a high level of access to resources. This isn't ideal for a wide variety of reasons, a big one in my opinion being that if the user leaves the organization then the organization will lose its abilities to do Legitify scans (not to mention the "what-if" of a PAT token of a user with this level of access becoming compromised).

Now... onto the good stuff...

Part of the initialization logic has a GraphQL query to get the viewerPermissions attribute of the repositories (see internal/clients/github/client.go). This attribute will always return null for a GitHub App per the GraphQL Object Docs. So great, I decided to just update that code locally to support looking for an empty string, and Legitify ran successfully... Sorta. The results felt a little incomplete, so I checked the permissions_log.json file and quickly found why it felt so incomplete:

{
  "missing_permissions": {},
  "skipped_policies": {
    "actions_can_approve_pull_requests": {
      "legitify-test": "Missing permission: admin:org"
    },
    "code_review_by_two_members_not_required": {
      "legitify-test": "Missing permission: repo"
    },
    "code_review_not_limited_to_code_owners": {
      "legitify-test": "Missing permission: repo"
    },

// --EDITED OUT A BUNCH IN THE MIDDLE TO SAVE SPACE SINCE THERE WAS A LOT--

    "users_allowed_to_bypass_ruleset": {
      "legitify-test": "Missing permission: repo"
    },
    "vulnerability_alerts_not_enabled": {
      "legitify-test": "Missing permission: repo"
    }
  }
}

That's a lot of skipped rules. I dug deeper into the rule analysis logic and stumbled into the skipper logic (internal/analyzers/skippers/skippers.go) and saw it is reading the token scopes. Hmm, how's it collecting the token scopes?

More digging led me to the collectTokenScopes function in the GitHub Client logic (internal/clients/github/client.go). Tl;dr what this is doing is issuing an empty post call to the GraphQL URL for GitHub, and looking for the X-OAuth-Scopes header to come back. It then parses that header which sets the scopes in the context to have the appropriate TokenScopes set to true or false.

With a classic PAT token, this header is there and looks something like this:

x-oauth-scopes: admin:org, admin:org_hook, read:repo_hook, repo

With a GH App (or a fine-grained PAT token) those OAuth scopes are absent. This means that all of the scopes get to set to false (it actually just doesn't update the initialScopes function output).

Also, it looks like OAuth Scopes are also used to define the required scopes for each rule (such as this one or this one or this one). The skipper is reading the policy metadata and looking for the requiredScopes, and then comparing it against the OAuth scopes it got back from the X-OAuth-Scopes header earlier (by way of reading the TokenScopes stored in the client Context object) to ensure that the required scopes are present on the token BEFORE attempting to do any logical validation to determine whether a rule is going to pass or fail. If the scope(s) aren't present, it'll skip the rule.

And that's just looking at the GitHub logic, I didn't even touch the GitLab logic (though I'd be willing to bet it's similar). AND since some of this logic is shared between GitHub and GitLab, that would make it an even larger refactor, which is riskier to the product.

@noamd-legit
Copy link
Contributor Author

Thank you for taking the time to write this detailed report @molson504x!
This is indeed the reason we don't support fine-grained and GitHub App Tokens currently.

The main problem is that when using GitHub API in many cases you can't know whether an API call failed to due insufficient permissions or because the resource doesn't exists. That caused a lot of problems in the beginning (because it showed many FAILED policies although the user just didn't have the required permissions) and why we decided the verify the scopes.

1.Since the GitHub API doesn't provide the scopes for other token types, I think the only option is to disable the distinction between FAILED and SKIPPED policies when running with a GitHub App Token.

  1. For fine-grained tokens, we'll need to disable the distinction and scope Legitify runs only to repository misconfigurations.

WDYT?

@molson504x
Copy link
Contributor

Hey @noamd-legit - thanks for reaching out. I agree, it would be nice if they distinguished between a "you can't do that" (a 401) versus "you're not allowed to do that" (403) and "this doesn't exist" (404). I've had my fair share of debugging nightmares because of that. That being said, I think you should still be able to do at least most of the organization-level scans with a GH App as long as it's got the right permissions. We just need to figure out what those permissions would need to be. And yes, odds are the GITHUB_TOKEN (from a GH Actions run) would probably still not work outside of the repository (and even for some things inside of the repo) but you can still just as easily use a GH App for accomplishing that task.

I took a quick look at the GitLab code and saw that it doesn't seem to verify scopes, and I was wondering if GitHub could work in a similar manner. My thought is that as long as the app has the appropriate permissions then it should be fine for doing policy scans. If the user passes in a token that doesn't have permissions to scan something, you'd know it pretty quickly by doing one of the pre-flight checks (similar to what's being done for an org-level scan). Assume a 404 means the organization cannot be scanned and either give a non-zero exit code (which will cause a fail condition in GH Actions) or log it in the error log and keep going on the ones which can be scanned.

I also found the area that does the /user/orgs API call, and looking at that code it felt a little over-engineered (though, I'm not sure exactly the reasoning behind it). It seems like that last call to the graphql endpoint would tell you everything you needed to know about whether or not the user has access to the org and what level of access they have. AND it would work for a GH App Token too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants