hackforla / website

Hack for LA's website
https://www.hackforla.org
GNU General Public License v2.0
323 stars 774 forks source link

ER: Create Epic to address GitHub permissions vulnerabilities #6649

Open gaylem opened 6 months ago

gaylem commented 6 months ago

Emergent Requirement - Problem

Right now, our permissions are set to read and write but this isn't best practice. According to GitHub, "It's good security practice to set the default permission for the GITHUB_TOKEN to read access only for repository contents. The permissions can then be increased, as required, for individual jobs within the workflow file."

Screenshot of current repo settings:

screenshot_2024-04-14_115940

The GITHUB_TOKEN is automatically created and available for use in your workflows without any extra setup. It can be used for lots of things, like PR automation, workflow triggers, and accessing the GitHub API. It's probably the right token for a lot of the things we do. However, it does have some limitations, such as restricted scopes for certain actions. For more sensitive operations or when more specific permissions are needed, you may need to use custom secrets with more finely tuned scopes. If we change it to read only instead of read/write, it will have an even more limited scope.

Additionally, we are setting specific permissions in some of our workflow files (Ex: codeql.yml). In some cases this may be redundant.

This epic and its issues should determine whether or not the permissions set in workflows are needed if we change the GITHUB_TOKEN scope to read only. Or, perhaps we will need to add additional write permissions. In other words, we need an audit of our permissions across our workflows to identify where we need to restrict or expand access for each workflow.

Issue you discovered this emergent requirement in

Date discovered

2024-04-15

Did you have to do something temporarily

Who was involved

@gaylem

What happens if this is not addressed

If this isn't addressed, there's an increased risk of unintended changes being made to the repository, potentially leading to data loss, security breaches, or other issues.

By following best practices and setting the default permission to read access, we will reduce the potential impact of any accidental or malicious actions, as the token will only have the ability to read repository contents. This minimizes the risk of unauthorized modifications and helps ensure the overall security of our GitHub workflows.

Resources

Recommended Action Items

Potential solutions [draft]

We need an issue to assess how we handle permissions in GitHub. This issue needs to accomplish the following:

  1. Identify all files that use more than read permissions
  2. Create an epic
  3. Add issues to the epic to modify permissions on each file individually and test locally

We need to be sure we won't introduce any breaking changes before flipping the repo setting back to read-only

roslynwythe commented 6 months ago

@gaylem Thank you for identifying the vulnerability and describing it so well. I'm applying ready for prioritization but can I also ask a question to help me understand:

gaylem commented 6 months ago

Hey @roslynwythe, great questions! Here's what I know:

  1. The GITHUB_TOKEN is automatically created and available for use in your workflows without any extra setup. It can be used for lots of things, like PR automation, workflow triggers, and accessing the GitHub API. It's probably the right token for a lot of the things we do. However, it does have some limitations, such as restricted scopes for certain actions. For more sensitive operations or when more specific permissions are needed, you may need to use custom secrets with more finely tuned scopes.
  2. I didn't add or change any permissions in codeql.yml, they where just there when I started working on the workflow. Here's where they are in the file: https://github.com/gaylem/hackforla-website/blob/7edb98bb0038f0bdc6ad2bb5545fe24b05433e79/.github/workflows/codeql.yml#L28-L31

I believe these permissions are just granting write permissions for security events. The read permissions may or may not be necessary.

What I'd like to examine as part of this epic is whether or not the permissions set in workflows are needed if we change the GITHUB_TOKEN scope to read only. Or, perhaps we will need to add additional write permissions. Basically, I'm proposing an audit of our permissions across our workflows to identify where we need to restrict or expand access for each workflow. Does that make sense?

roslynwythe commented 6 months ago

Thank you @gaylem that sounds valuable.

ExperimentsInHonesty commented 6 months ago

@gaylem Please resolve the comments and update the top if applicable. Once all the comments are hidden, then add the ready for prioritization label again so I can review.

github-actions[bot] commented 6 months ago

Hi @gaylem, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:- i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?) ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)

ExperimentsInHonesty commented 6 months ago

@gaylem

github-actions[bot] commented 6 months ago

Hi @gaylem, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:- i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?) ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)

gaylem commented 6 months ago

Hey @ExperimentsInHonesty, I don't think that testing will be impacted by this issue and so #6402 shouldn't be a dependency.

The only way testing might be impacted is if we have to create new tokens or change any scopes. That may not happen, though, and it doesn't make sense to delay updating our overall GHA instructions to wait for this.

Personally, I think updating the instructions is a more pressing issue and would rather make some minor tweaks later on if our audit reveals a need for token changes.

Let me know what you think. Thanks!

ExperimentsInHonesty commented 5 days ago

Make the following issues

HackforLABot commented 5 days ago

Hi @dcotelessa, thank you for taking up this issue! Hfla appreciates you :)

Do let fellow developers know about your:- i. Availability: (When are you available to work on the issue/answer questions other programmers might have about your issue?) ii. ETA: (When do you expect this issue to be completed?)

You're awesome!

P.S. - You may not take up another issue until this issue gets merged (or closed). Thanks again :)