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

Update GHA token permissions for generic container workflow #1258

Merged
merged 6 commits into from
Nov 28, 2022

Conversation

ianlewis
Copy link
Member

Fixes #1256
Fixes #1257
Updates #547

  1. Sets top level token permissions for the container generator workflow.
  2. Updates container generator workflow to no longer require package: write permissions on the GHA token. Instead we require users of ghcr.io to login using a PAT.

Signed-off-by: Ian Lewis <ianlewis@google.com>
Signed-off-by: Ian Lewis <ianlewis@google.com>
Signed-off-by: Ian Lewis <ianlewis@google.com>
Signed-off-by: Ian Lewis <ianlewis@google.com>
@ianlewis ianlewis mentioned this pull request Nov 24, 2022
@ianlewis ianlewis marked this pull request as ready for review November 24, 2022 02:46
Signed-off-by: Ian Lewis <ianlewis@google.com>
@ianlewis ianlewis self-assigned this Nov 28, 2022
@laurentsimon
Copy link
Collaborator

laurentsimon commented Nov 28, 2022

Note that it's debatable that long-lived PAT tokens are a more secure option to ephemeral GITHUB_TOKEN. But we can't have both. Starting as you suggest in this PR means we can later add a package:write if needed.
We need to follow-up whether they can compute the permissions for a reusable workflow dynamically at runtime - instead of statically.

@ianlewis
Copy link
Member Author

ianlewis commented Nov 28, 2022

Note that it's debatable that long-lived PAT tokens are a more secure option to ephemeral GITHUB_TOKEN. But we can't have both. Starting as you suggest in this PR means we can later add a package:write if needed. We need to follow-up whether they can compute the permissions for a reusable workflow dynamically at runtime - instead of statically.

Yeah, I understand that. This at least makes it so you don't have to give packages: write permissions even if you're using a repository other than ghcr.io. Hopefully this will be fixed by the more granular PAT tokens that are in beta. I'm pretty sure permissions can't be computed dynamically.

@ianlewis
Copy link
Member Author

BTW, I tried to pass the GITHUB_TOKEN with package:write permission to the workflow as an input, basically giving the token more permissions than the workflow asked for, but that didn't seem to work. It still required the workflow to request packages:write for it to work.

@laurentsimon
Copy link
Collaborator

laurentsimon commented Nov 28, 2022

Hopefully this will be fixed by the more granular PAT tokens that are in beta.

the fine-grained tokens seem to be scoped in time and always expire (at least in the beta), which makes them unsuitable for workflows. (We can't expect uses to update PATs every month or so)

I'm pretty sure permissions can't be computed dynamically.

I just meant that GH should be able to check the permissions given by the caller at runtime on access to actions that need these permissions. Technically they should be able to do that, without doing a validation statically.

@ianlewis
Copy link
Member Author

the fine-grained tokens seem to be scoped in time and always expire (at least in the beta), which makes them unsuitable for workflows. (We can't expect uses to update PATs every month or so)

Yeah, that seems odd if they allow more granular scopes, users should be more comfortable with having long-term tokens. Maybe that's some feedback for GitHub...

I just meant that GH should be able to check the permissions given by the caller at runtime on access to actions that need these permissions. Technically they should be able to do that, without doing a validation statically.

Yeah, I explicitly tried that and it didn't seem to work. I set package: write on the call to the reusable workflow and passed the GITHUB_TOKEN as an input, but still got permissions errors. It worked when the re-usable workflow requested packages: write so it seems like the workflow needs to request all the permissions it needs to run in all cases, even if it uses only a subset.

The only solution I can think of is to make a separate workflow just for ghcr.io that requests packages: write but that feels like overkill...

@ianlewis ianlewis merged commit 38f9f24 into slsa-framework:main Nov 28, 2022
ianlewis added a commit to ianlewis/slsa-github-generator that referenced this pull request Nov 29, 2022
…lsa-framework#1258)"

This reverts commit 38f9f24.

Signed-off-by: Ian Lewis <ianlewis@google.com>
@ianlewis ianlewis mentioned this pull request Nov 29, 2022
ianlewis added a commit that referenced this pull request Nov 29, 2022
* Revert "Update GHA token permissions for generic container workflow (#1258)"

This reverts commit 38f9f24.

Signed-off-by: Ian Lewis <ianlewis@google.com>

* set default top level perms

Signed-off-by: Ian Lewis <ianlewis@google.com>

Signed-off-by: Ian Lewis <ianlewis@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants