runner icon indicating copy to clipboard operation
runner copied to clipboard

A way to manage access around who can edit contents of `.github/workflows/`

Open timharris777 opened this issue 5 years ago • 57 comments

Enhancement: Provide role separation around who can edit contents of .github/workflows/

Reasoning: In an organization setting you have a lot of people who have write access to repositories. Add github actions workflows and secrets manager (think org level secret manager when it is released) and every person with write access could change a workflow file or create a new workflow file to print out the contents of a secret. Being able to limit the people who can edit workflows would be a huge plus in the security front.

@ds0440 @josephshanahan-cfa

timharris777 avatar Apr 28 '20 18:04 timharris777

@chrispat from the product team for feedback.

TingluoHuang avatar May 19 '20 20:05 TingluoHuang

@timharris777 limiting of editing the workflow file will not help with security. If you are running any code as part of your build and someone can change that code they can potentially export secrets by dumping process memory.

chrispat avatar May 20 '20 12:05 chrispat

@chrispat, what you are saying is a helpful callout, but please consider this:

The scope of a secret is limited to a step.

If a secret's scope is limited to a step, then secrets are isolated and contained to that specific step. For example, in our build process the only step that would allow someone to edit it and dump process memory (as you said) is a step that does a ./gradlew build, and this step contains zero secrets. This step builds an artifact and then passes it to another step that deploys the artifact. Only the step that deploys has access to a secret.

This makes me think the enhancement request should also ask for role separation around who can edit contents of the entire .github/ folder. Anything in this directory would be protected from unprivileged editing and the malicious dumping of process memory. Since the .yml files are in this folder and would also be protected, it would not be possible for someone to edit the steps and change the scope of secrets.

josephshanahan-cfa avatar May 20 '20 14:05 josephshanahan-cfa

@josephshanahan-cfa the scope of a secret is a job not a step meaning that the secret is active in the memory of the runner for the scope of the job in which it is referenced.

Your gradle example is a great one here as it runs code that is part of the repo that anyone with write access to the repo can modify. This means that they could add something to dump the process memory for the runner which has all of the secrets being used in that job. Have a separation of roles is false security when you are talking about config as code and CI that runs arbitrary code.

chrispat avatar May 20 '20 14:05 chrispat

So you are telling me that any 3rd party action can dump all of my secrets, regardless of the step? Could you provide an example for how to do this across steps as you mentioned?

josephshanahan-cfa avatar May 20 '20 15:05 josephshanahan-cfa

I am saying that any code you run during your CI could potentially dump all of the secrets that are referenced in the execution scope which is a job. If you are running your job in a container this becomes significantly harder because the runner is not inside that container. However, for platforms like MacOS that don't support containers at all or on Windows where containers are not practical for many CI needs that is not an option.

chrispat avatar May 20 '20 15:05 chrispat

Thanks for the response @chrispat, could you please provide an example for how to access secrets across steps in an ubuntu environment that is not using containers?

josephshanahan-cfa avatar May 20 '20 15:05 josephshanahan-cfa

@josephshanahan-cfa I am not sure what you mean. Are you asking for an example of how you would dump process memory and then read the strings out of it?

Here is an article on how to dump the process memory of a .NET process https://devblogs.microsoft.com/dotnet/collecting-and-analyzing-memory-dumps/

chrispat avatar May 20 '20 15:05 chrispat

Yes, that would be helpful. From the GitHub Actions docs it says:

A job contains a sequence of tasks called steps. Steps can run commands, run setup tasks, or run an action in your repository, a public repository, or an action published in a Docker registry. Not all steps run actions, but all actions run as a step. Each step runs in its own process in the runner environment and has access to the workspace and filesystem. Because steps run in their own process, changes to environment variables are not preserved between steps. GitHub provides built-in steps to set up and complete a job.

https://help.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobsjob_idsteps

This documentation makes it sound like the step runs in its own process. It sounds like you are saying that steps are sharing process memory.

Could you give me an example as you mentioned of dumping process memory and then reading strings out of it? Specifically, where I would be able to see the secrets declared in one step, in another step (All in the same job).

josephshanahan-cfa avatar May 20 '20 15:05 josephshanahan-cfa

Steps are run in their own process space and its own process memory, but, they are all run under the same user context and as a user you can read the process memory of other process running in your context. This is essentially how a debugger works, you run the debugger as you and attach to another process. If you are an admin on the machine you can read the memory of essentially any process on the machine.

I don't have a working example right now but I have seen it done.

chrispat avatar May 20 '20 15:05 chrispat

@chrispat is correct. Essentially it's user code running on a machine as admin. Not really securable. Which means you have to trust who can edit your workflow file or in the case of something like deploys etc, your workflows and scripts could be in another repo than the code / containers being built and tested independently by developers. This is also why we don't send secrets to forks.

bryanmacfarlane avatar May 20 '20 16:05 bryanmacfarlane

Also one should be critical to actions found on the marketplace. These could steal information and/or do bad actions in general.

davidkarlsen avatar May 20 '20 16:05 davidkarlsen

Hey guys, we understand that anyone who can edit the workflow file can possibly dump secret contents.

Our ask is that you provide role based separation of who can edit the contents of the .github/ folder or the .github/workflows folder. For example, looking at the current roles of read, triage, write, maintain, and admin-- maybe the maintain and admin roles could have access to update the folders in question. This would prevent everyone who only has write access to the repo from being able to edit the workflows.

Separately, Gitlab allows per folder permissions in addition to repo level read/write permissions which would solve our issue but Github does not seem to have this capability. Is that on the radar at all?

timharris777 avatar May 21 '20 20:05 timharris777

There is the CODEOWNERS feature, https://help.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners which you can utilize to apply branch protection rules on a directory, e.g. .github/workflows/ but this is limited to being applied as a review requirement on Pull Requests.

This would block a Pull Request subject to review by the code owner(s), but if an individual has write access it would not block them just committing directly to the branch.

peter-murray avatar Jun 16 '20 15:06 peter-murray

Yeah, CODEOWNERS is not quite what we are looking for as someone can create a branch and a workflow file that will trigger in that branch without any kind approval.

timharris777 avatar Jun 18 '20 13:06 timharris777

Any updates on this? I have engineers in my org asking about it. CODEOWNERS is not a valid solution. I say this because any engineer with write access to the repo can create a branch and have a workflow fire off of pushes to the new branch.

@headljdghub, feel free to provide your thoughts as well.

timharris777 avatar Aug 19 '20 16:08 timharris777

Another security concern we have run across with any engineer having an "editor" role and the ability to branch and create/run workflows off of that branch is the following:

Let's say we have a self-hosted runner running in an AWS account, Azure account, or Google account. This repo has access to use that self-hosted runner in order to deploy the application. If we have 200+ developers working on this application then essentially all of those developers have access to do something in the AWS/Azure/Google account via creating their own workflows in a separate branch and having it trigger off of a push event to that specific branch.

This would not be a problem if we could limit what users are able to push to the .github/workflows directory.

Again, gitlab covered this with their folder level permissions. Is this on github's radar or roadmap?

@mikedrexler

timharris777 avatar Aug 19 '20 18:08 timharris777

@timharris777 Thanks for raising the issue with the .github/workflows protection as it relates to self-hosted runners. This particular issue has prevented our team from using GHA and self-hosted runners more broadly.

headljdghub avatar Aug 19 '20 18:08 headljdghub

Hi Team, could you use CODEOWNERS in the root of .github to ensure only an agreed upon group could approve a push to master for .github/workflows directory. Have a look at the guidance at https://docs.github.com/en/github/creating-cloning-and-archiving-repositories/about-code-owners and give me your thoughts?

cc/ @headljdghub @timharris777

mikedrexler avatar Aug 19 '20 19:08 mikedrexler

@mikedrexler The actions on non-master branches would still have access to the self-hosted runners to execute whatever they wanted to. What we are trying to accomplish is to limit the number of people who have access to the privileged self-hosted runners by restricting access to the .github/workflows folder. We would be using the self-hosted runners to deploy applications and create necessary infrastructure so the runner will be pretty highly privileged.

headljdghub avatar Aug 19 '20 20:08 headljdghub

Thank you @headljdghub for the additional context!

Team, thank you in advance for your 👀 on this!

mikedrexler avatar Aug 19 '20 20:08 mikedrexler

Any word on this?

  1. We have looked at CODEOWNERS and it is not a solution to protect who can create and trigger workflows. Any workflow file in the pull request can be run if triggers are set correctly.
  2. We have looked at adopting the fork and submit a pull request back method of contributing and it is also not a solution to protect who can create and trigger workflows. If we enable actions for fork pull requests essentially any workflow in the pull request itself can be run if triggers are set correctly..

The following are possible solutions:

  1. https://github.com/actions/runner/issues/494#issuecomment-663871697
  2. Adding folder/file level permissions in addition to repo level permissions

Related Issues:

  • https://github.com/actions/runner/issues/494
  • https://github.com/actions/runner/issues/631

timharris777 avatar Sep 02 '20 19:09 timharris777

Hi guys, I just happened to run across this: https://github.com/github/roadmap/issues/111

@mikedrexler, do you know anything about this? Would love to find out more from the product owner. It's possible this would solve our issue.

timharris777 avatar Sep 03 '20 12:09 timharris777

The mentioned roadmap item will not help you in this regard, there is nothing in that feature that will provide limits to editing file under a path in a repository.

The feature will allow you to take a role like say read access and add a permission like create issue to craft a new role that you can assign to users.

peter-murray avatar Sep 03 '20 12:09 peter-murray

Thanks @peter-murray. Wasn't sure if a new permission would be introduced allowing something similar to this: https://github.com/actions/runner/issues/494#issuecomment-663871697

timharris777 avatar Sep 03 '20 12:09 timharris777

Any updates on this? This is a big issue for adoption for anything CD related with GHA.

mihkelparna1 avatar Oct 06 '20 07:10 mihkelparna1

It will be good if we can just force a repo to use a workflow file from another repo so that we can easily protected the workflow file repo without fine grained permission control on the main repo.

xlc avatar Oct 13 '20 00:10 xlc

It will be good if we can just force a repo to use a workflow file from another repo so that we can easily protected the workflow file repo without fine grained permission control on the main repo.

@xlc This is just like (our) old style CI where the "build steps" are defined elsewhere (in Jenkins and in MS TFS Build) and not in a file (.github/workflows, appveyor.yml, Jenkinsfile, travis.yml, circleci.yml, ...) in the branch being checked out.

Having the workflows defined elsewhere can also mean another branch. Similar to what you have with a specific ghpages branch.

Side-note: Limiting what actions to allow does not help much here (https://docs.github.com/en/free-pro-team@latest/github/administering-a-repository/disabling-or-limiting-github-actions-for-a-repository#allowing-specific-actions-to-run). It doesn't prevent creation of a workflow where a secret is posted somewhere using a simple curl command (one do need to know the name of the secrets though).

How would these two suggestions do?

Problem: Allowing GitHub Actions allows anyone with write access to create a pull request with their own workflow Solution: In the repo Actions settings where you control what actions to allow, add the following to constrain the source for the workflow files being run:

Screenshot 2020-10-13 at 11 48 03

Problem: Having only one branch where you can make changes to workflows disallows development of workflows in PR branches Solution: Use branch protections and add a checkbox for overriding the constraint defined above:

Screenshot 2020-10-13 at 11 59 39

Note: This would allow one to have long-lived branches like main and workflows-dev for example. If short-lived branches (using pull requests) are desired then one could use a naming pattern such as actions-* or workflowchanges-* to lessen the burden added to maintain branch protection rules to allow workflow changes.

What do you think @peter-murray ? Would that work with your setup @mihkelparna1 ?

anderssonjohan avatar Oct 13 '20 10:10 anderssonjohan

Actually I find the GitLab Solution on this front pretty smart. In the Secret Defitions ("CI/CD Variables" call in GitLab) you can tick a box "Protected", with protected = Protected variables are only exposed to protected branches or tags.

That means, anybody can mess around in the Pipeline (GitLab Wording for Actions) of his branch, but will not have access to the secrets to actually do something "nasty". These are only available when the branch is merged into a protected branch.

beckerjohannes avatar Oct 20 '20 13:10 beckerjohannes

Actually I find the GitLab Solution on this front pretty smart. In the Secret Defitions ("CI/CD Variables" call in GitLab) you can tick a box "Protected", with protected = Protected variables are only exposed to protected branches or tags.

That means, anybody can mess around in the Pipeline (GitLab Wording for Actions) of his branch, but will not have access to the secrets to actually do something "nasty". These are only available when the branch is merged into a protected branch.

But this doesn't fill the bill, at least not in the scenario where you want to receive PRs and run your workflow with secrets, using the code submitted in the PR branch. We need the secrets to do a proper build but don't want the submitter to be able to modify the workflow itself.

anderssonjohan avatar Oct 20 '20 17:10 anderssonjohan