slsa-github-generator icon indicating copy to clipboard operation
slsa-github-generator copied to clipboard

`id-token: write` not available on pull_requests

Open ianlewis opened this issue 3 years ago • 11 comments
trafficstars

Adding id-token: write to workflows triggered by pull_request events doesn't seem to be allowed.

This seems a bit problematic that we would expect that folks only run our workflow when a tagged. I can imagine folks would run into issues if they only exercise the workflows when releasing new versions as there would be significant time and changes to their repo in between releases. If the builders don't run on pull_request it will be hard to ensure that new code works with the reusable workflows.

For comparison, go-releaser's example workflow runs on pull_request. https://github.com/goreleaser/goreleaser-action#workflow

ianlewis avatar May 30 '22 13:05 ianlewis

Related #124

ianlewis avatar May 30 '22 13:05 ianlewis

Goreleaser works on PR but they cannot sign with keys, because the keys need to be stored in secrets that are not available on PRs.

To have access to the secrets or OIDC tokens, we could support pull_request_target instead.

@ianlewis are you suggesting we support PRs and not output provenance in this case (to be on par with Goreleaser)?

laurentsimon avatar May 31 '22 22:05 laurentsimon

@laurentsimon I'm not actually sure what's best right now. I just think we probably need to support PRs in some capacity even if we don't sign provenance because it would introduce differences between how building in PR pre-submits work and how it works on releases. I think it may be a future point of user friction.

Like for example, maybe just running the builder on PRs is the right way to go?

ianlewis avatar May 31 '22 22:05 ianlewis

I think that's fine, yes. I considered this to be low priority for v1, and wanted to see if we get push back from people. We can tackle this in the next iteration. Wdut?

laurentsimon avatar Jun 01 '22 00:06 laurentsimon

I think that's fine, yes. I considered this to be low priority for v1, and wanted to see if we get push back from people. We can tackle this in the next iteration. Wdut?

I agree. I haven't included it in the v1 milestone.

ianlewis avatar Jun 03 '22 02:06 ianlewis

As an aside, we currently use the NilClientProvider for pull requests which returns nil for both the OIDC and GitHub clients, but we should still be able to use the GitHub API in pull requests as it uses github.token for auth. https://docs.github.com/en/actions/security-guides/automatic-token-authentication#permissions-for-the-github_token

That should allow us to set the invocation entry point properly and not have special code for that.

ianlewis avatar Jun 03 '22 02:06 ianlewis

+1, we can do that.

laurentsimon avatar Jun 03 '22 15:06 laurentsimon

We can also look into supporting the pull_request_target event which may allow us to get the right permissions because it runs using workflow code at the base ref of the PR.

ianlewis avatar Jun 09 '22 04:06 ianlewis

agreed, that's the way to support pull requests if we need to.

laurentsimon avatar Jun 09 '22 14:06 laurentsimon

thinking more about this, it may be more complicated than it seems. The OIDC token will contain information from the repo at HEAD: repo name, commit hash, etc, not the PR's repo's information.

laurentsimon avatar Jun 15 '22 02:06 laurentsimon

Related #358 which tracks support overall.

ianlewis avatar Jun 20 '22 07:06 ianlewis