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

Relax token permission check or not #1129

Closed
laurentsimon opened this issue Oct 13, 2021 · 3 comments
Closed

Relax token permission check or not #1129

laurentsimon opened this issue Oct 13, 2021 · 3 comments
Labels

Comments

@laurentsimon
Copy link
Contributor

laurentsimon commented Oct 13, 2021

When calculating the score for the token permission check, we reduce the score when certain dangerous permissions are used, see permissions.go#L262

An alternative implementation would be to only check whether the permissions are defined explicitly or no via permissions: keyword. There are pros and cons to both approaches.

Checking only for the presence of permission definition permissions: is simpler but does not catch overly-broad permissions (e.g. if a user declared all permissions are write).

Reducing score based on dangerous permissions is more granular but prone to false positive, and will likely never be 100% accurate, see #1099.

@naveensrinivasan
Copy link
Member

naveensrinivasan commented Oct 14, 2021

@laurentsimon IMO some detailed notes on what needs to be fixed would be helpful for a good first issue, Thanks

@laurentsimon laurentsimon added needs discussion and removed good first issue Good for newcomers labels Oct 14, 2021
@laurentsimon laurentsimon changed the title Relax token permission check Relax token permission check or not Oct 14, 2021
@laurentsimon
Copy link
Contributor Author

laurentsimon commented Oct 14, 2021

@laurentsimon IMO some detailed notes on what needs to be fixed would be helpful for a good first issue, Thanks

I've updated the description. I meant to tag this as needs discussion, not good first issue. I've updated the label too. Thanks for catching the mistake.

@laurentsimon
Copy link
Contributor Author

We discussed last meeting. We will separate out data/result gathering and policy/score calculation. So we'll keep our calculation as it is today and move to the policy calculation over time.

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

No branches or pull requests

2 participants