[bug] Problem using SLSA3+ provenance generator for arbitrary projects

[rbehjati]

Describe the bug
I am following the instructions for using generator_generic_slsa3 to generate provenances for Oak. The call to the workflow fails with the following error:

Run ./.github/actions/generate-builder/generate-builder.sh
/home/runner/work/_temp/9c89bedb-16a3-4329-be6c-a854f00c4572.sh: line 1: ./.github/actions/generate-builder/generate-builder.sh: No such file or directory
Error: Process completed with exit code 127.

The workflow is running and failing on a pull_request trigger. I know that pull_request triggers are not supported, but since this is the first time I am using this workflow, I was hoping to experiment with it before merging this PR to main. If this is the expected behaviour on pull requests, it would help to have the behaviour described in the documentation.

To Reproduce
Here is the PR that uses the generator_generic_slsa3 workflow: project-oak/oak#3166
This is the failed action: https://github.com/project-oak/oak/runs/7953028722?check_suite_focus=true. The step that fails is the "Generate builder" step.

Screenshots

[asraa]

It is true that this happens because of the pull_request trigger. Because of this, the workflow can't access the OIDC token required to create the Fulcio issued certificate.

Can we support a dry-run option that is only valid for pull_request triggers?

It depends what you would like to test here: If you want to test provenance generation, but not signature validation, we can do something similar to how the CI tests here work in which they do not actually create sigs.

[rbehjati]

A dry-run option would be great. For the pull-request, I only care about provenance generation.

[asraa]

That would be an easy fix, we can modify IsPresubmitTests():

slsa-github-generator/internal/utils/presubmits.go

Line 20 in 1a55da8

func IsPresubmitTests() bool {

@laurentsimon any oppposition to allowing all pull_request triggers here?

[laurentsimon]

pull_request is already listed, is it not?

[asraa]

pull_request is already listed, is it not?

Must be pull_request AND this repository, I can change to remove the AND

[laurentsimon]

Got it. Is this something we should give an option for so that's it's more explicit? I'm worried users won't realize that signature fails.

/cc @ianlewis

[asraa]

Yes, exactly: only if a dry-run input option is passed in.

[laurentsimon]

That'd work for me. So the dry-run would generate the signatures on triggers that support it, and not for triggers like pull_request, or no generation in both cases?

[asraa]

So the dry-run would generate the signatures on triggers that support it, and not for triggers like pull_request, or no generation in both cases?

I was thinking yes, that it would end up making the workflow compatible with pull_request when run in that.

I agree there's a risk that users may assume this would be valid provenance. No upload or no new release would be done in the case that a dry run is being performed.

[ianlewis]

I think having a dry-run option is fine if we document it. We should also probably put in some kind of warning in the output that the provenance is not signed.

I'm thinking:

• Add dry-run option to skip signing on pull_request events
  • Print a warning "Provenance is not signed in a dry run." in this case.
• Print a friendlier error message run in a pull_request without the dry-run option.

[rbehjati]

Got it. Is this something we should give an option for so that's it's more explicit? I'm worried users won't realize that signature fails.

Does the signature actually fail, or is it possible to skip it? My assumption was that the provenance is first generated, and then signed in a separate, possibly optional step. If the two must happen together, perhaps the dry-run does not even need to generate the provenance, but just give a signal the the workflow job is set up correctly. For instance, for my PR, what I want to know is that I have configured the permissions correctly, and am passing in the right inputs.

For the permissions, by the way, I had to change contents: read to contents: write.

Also, the current error message says ./.github/actions/generate-builder/generate-builder.sh: No such file or directory. That does not look like a permission or access problem to me.

[laurentsimon]

Got it. Is this something we should give an option for so that's it's more explicit? I'm worried users won't realize that signature fails.

Does the signature actually fail, or is it possible to skip it? My assumption was that the provenance is first generated, and then signed in a separate, possibly optional step. If the two must happen together, perhaps the dry-run does not even need to generate the provenance, but just give a signal the the workflow job is set up correctly.

Would using the workflow_dispatch trigger solve the problem? You need to merge it in first, but you can then trigger it manually. That will catch permission issues or other problems. You can then download the provenance from the artifacts (when viewing the Action run).

For the permissions, by the way, I had to change contents: read to contents: write.

We fixed the documentation yesterday.

[rbehjati]

Would using the workflow_dispatch trigger solve the problem?

Good idea. I'll try that, and will update this issue with the result.
I think a dry-run option is still a better solution :)

We fixed the documentation yesterday.

Nice. Thanks.

[rbehjati]

Thanks for the help. I was able to use the workflow and generate a provenance. My first attempt failed, because I had not set the subject correctly. That is the kind of issue that a dry-run option can detect :)

Should I close this bug, or do you prefer to keep it open for more discussion about a dry-run option? Perhaps #358 is a better place for that discussion?

[laurentsimon]

Let's keep here. Dry-run option is more general than the pull_request. Thanks again for the issue

[ianlewis]

Related #131 #358 #124