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

[bug] detect-env fails on pull-request, only works on tag #1527

Closed
davidism opened this issue Jan 12, 2023 · 8 comments
Closed

[bug] detect-env fails on pull-request, only works on tag #1527

davidism opened this issue Jan 12, 2023 · 8 comments
Labels
status:triage Issue that has not been triaged type:bug Something isn't working

Comments

@davidism
Copy link

davidism commented Jan 12, 2023

While setting up the generic generator workflow for MarkupSafe, I used the on: pull-request: trigger so that I could debug it by pushing more to the PR. However, it failed because the detect-env job returned my repository rather than the generator repository. When I switched to on: push: tags, the job succeeded.

Documentation doesn't seem to indicate that only tags are supported, and looking at the detect-workflow source shows that it should work with pull-request:

if eventName == "pull_request" {

Here's the output of detect-env on a failed run triggered by a pull request event: https://github.com/davidism/markupsafe/actions/runs/3904391874/jobs/6670129941#step:3:7, it returns davidism/markupsafe.

And here's the output on a successful run triggered by a tag instead: https://github.com/davidism/markupsafe/actions/runs/3904749127/jobs/6670933961#step:3:7, it returns slsa-framework/slsa-github-generator.

It was also very hard to debug this because the output was not helpful. It showed the detect-env job passing, then the generator job failed with the error Can't find 'action.yml', 'action.yaml' or 'Dockerfile' under '/home/runner/work/markupsafe/markupsafe/__BUILDER_CHECKOUT_DIR__/.github/actions/privacy-check'. Did you forget to run actions/checkout before running your local action? https://github.com/davidism/markupsafe/actions/runs/3904391874/jobs/6670142285#step:2:239

@davidism davidism added status:triage Issue that has not been triaged type:bug Something isn't working labels Jan 12, 2023
@davidism
Copy link
Author

davidism commented Jan 12, 2023

It would be very helpful if the pull-request event worked, because I build artifacts for the same library version when a new version of Python comes out, which doesn't require a new tag or release of the library.

cc @sethmlarson

@ianlewis
Copy link
Member

Hi @davidism,

Our lack of support for pull requests is due to a limitation in the permissions given to GitHub tokens for pull_request events (#131). We need id-token: write in order to generate OIDC tokens to use for signing provenance but that isn't available for pull requests. You can follow support for pull_request events on #358.

Documentation doesn't seem to indicate that only tags are supported,

We do have some documentation about supported triggers/events here. Let us know if there is something we can do to make those docs more discoverable.

and looking at the detect-workflow source shows that it should work with pull-request:

It is a bit confusing since most of the pull request logic there is to help the workflows work with our own pre-submit tests. I'll see if we can't improve those error messages.

It would be very helpful if the pull-request event worked, because I build artifacts for the same library version when a new version of Python comes out, which doesn't require a new tag or release of the library.

I'm curious why you need to do this. Is it necessary to do this to support a new version of Python?

@davidism
Copy link
Author

Sorry about that, I totally missed that pull_request was called out as not working.

I'll explain Python builds a bit. Unless you're using the special abi3 mode (MarkupSafe can't), you need to build a platform wheel for each Python X.Y version, OS, and architecture triple, which requires running the build on each combo. The cibuildwheel project automates this. There are no actual code changes needed to build against a new Python version, only the latest version of cibuildwheel. So if Python 3.12 comes out, I can just create a PR to update cibuildwheel, which would trigger the build workflow, then upload the new 3.12 artifacts for the existing tag/version.

But I also missed that the generator does support workflow_dispatch, which is an acceptable alternative to pull_request for me, if I can figure out how to get it to run against a specific tag/commit in that case.

@asraa
Copy link
Collaborator

asraa commented Jan 13, 2023

That being said, I do think that as we migrate to v1.0 provenance and use our internal signing action, we will likely be able to support pull_request for generating unsigned attestations (e.g. skip the signing action and just publish it's input -- the raw attestation).

@asraa
Copy link
Collaborator

asraa commented Jan 13, 2023

To run against a specific commit/tag in workflow_dispatch you can add arguments to checkout here https://github.com/davidism/markupsafe/actions/runs/3904391874/workflow#L8 that uses a workflow dispatch input for the commit or tag you want to run against.

@laurentsimon
Copy link
Collaborator

If you can use worflow_dispatch that would be ideal.

There's also pull_request_target trigger that has write permissions access to tokens, but you have to be careful that only your PRs can run, otherwise you expose yourself to letting external contributors hijack the flow and build what they want, and also read your GH encrypted secrets -- see some discussion at sigstore/sigstore-conformance#55

I would recommend not using pull_request_target: additional risk, additional process for you to review PRs, etc.

@davidism
Copy link
Author

workflow_dispatch is working for me now, thanks for the answers.

@laurentsimon
Copy link
Collaborator

Thanks @davidism for reaching out. Closing the issue then.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:triage Issue that has not been triaged type:bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants