Generate build provenance before upload.

[josephsweeney]

This PR adds a step in the post-build action that generates build providence for bottles that are built and will be uploaded. This is part of the larger effort to add verifiable build provenance for Homebrew bottles.

This is currently a draft and needs some further testing. Any feedback in the meantime is encouraged! Please let me know if there is a problem with signing post-build and before upload, or if there is a better place for this. My work so far leads me to believe this is the simplest place, but I'm certain others have more context that I am lacking.

[request-info[bot]]

Please provide a better issue/pull request title and/or description!

[MikeMcQuaid]

@josephsweeney 👋🏻 can you fill out the PR description please? Thanks!

[josephsweeney]

Of course, added a description. This draft PR may have been a bit premature, as I want to test it a bit more. Though it is small and fairly easy to understand. Please let me know if any clarification is needed!

[Bo98]

The weird thing about pre/post-build unlike most other actions in this repo is it is tightly coupled with homebrew-core CI to the point that moving it here has meant we no longer actually have CI to test PRs that change these steps.

I'm ok however to just merge and see what happens given a revert would take seconds.

[Bo98]

Please let me know if there is a problem with signing post-build and before upload, or if there is a better place for this. My work so far leads me to believe this is the simplest place, but I'm certain others have more context that I am lacking.

Haven't looked into what credentials are needed here but just to note: the post-build action runs in a pull_request context, including fork PRs, so any necessary token permissions may be restricted accordingly.

[MikeMcQuaid]

The weird thing about pre/post-build unlike most other actions in this repo is it is tightly coupled with homebrew-core CI to the point that moving it here has meant we no longer actually have CI to test PRs that change these steps.

I wonder if actions only used by Homebrew/homebrew-core should live in that repo for that reason? Thoughts @carlocab?

I'm ok however to just merge and see what happens given a revert would take seconds.

Fine with me 👍🏻

[carlocab]

I wonder if actions only used by Homebrew/homebrew-core should live in that repo for that reason? Thoughts @carlocab?

Not opposed to that. Alternatively, we can just add tests directly here, but then we would need to make sure that the tests here don't drift from actual usage in Homebrew/core.

[Bo98]

I wonder if actions only used by Homebrew/homebrew-core should live in that repo for that reason?

Composite actions that will only be used by one repo should probably use the workflow_call system. Keeps the benefits of reusability without the extra maintenance cost of moving it off-repo.

Portable Ruby repo has an example of this where it reuses build steps between the regular PR CI and release workflows.

[josephsweeney]

The weird thing about pre/post-build unlike most other actions in this repo is it is tightly coupled with homebrew-core CI

Homebrew core CI is actually where this needs to run, so that is why I added it here.

the post-build action runs in a pull_request context, including fork PRs, so any necessary token permissions may be restricted accordingly

Yeah, I have tested in a workflow from a from of homebrew-core and it seems to work but I think it needs to actually be merged to know for sure.

Is there any reason to (or reason not to) also sign the manifests that are generated alongside the bottles?

I believe we mainly care about the build provenance for the tarballs, as they contain what will actually be executed. I will be thinking more about the exact threat model, but we can also add signing of the manifests pretty simply in the future so I think it is best to just sign the tarballs for now.

Composite actions that will only be used by one repo should probably use the workflow_call system.

There is an argument that this is a more general feature as anyone that reuses this action gets build provenance out of the box if they choose. That being said, I do think there is a case for bringing this action's functionality into homebrew-core if that is the sole intended user (and what I am adding this feature for).

Though, I think there may have been a good reason for putting all of Homebrew's actions into a separate repository. I want to respect Chesterton's fence.

[Bo98]

Though, I think there may have been a good reason for putting all of Homebrew's actions into a separate repository. I want to respect Chesterton's fence.

Yeah wasn't meant to be anything to action here in this PR - just brought up a point that we should probably address on our side in the future.

If you see the whole action move about in the future that will be why.

[woodruffw]

I believe we mainly care about the build provenance for the tarballs, as they contain what will actually be executed. I will be thinking more about the exact threat model, but we can also add signing of the manifests pretty simply in the future so I think it is best to just sign the tarballs for now.

To add some additional context here: there is no reason why we couldn't also sign for the manifests, although verifying the resulting signatures is slightly more challenging -- since the manifest is arbitrary JSON, we'd need to determine which properties in it constitute a "sufficiently" valid/authentic manifest. This is different from the bottle case, where matching the bottle name and contents is sufficient for authenticity (post signature verification).

That's not an insurmountable problem by any means, but it's slightly less well defined than the bottle verification case and has implications for the manifest's stability (i.e., properties needed for validation can't easily be changed without a transition process). So TL;DR: we should do this, but it needs some additional design thought 🙂

[MikeMcQuaid]

Let's try this out. @Homebrew/maintainers feel free to revert this ASAP if there's any issues.

Thanks @josephsweeney!

[p-linnane]

All pipelines are failing:

Error: Bad request - github-early-access/generate-build-provenance@main is not allowed to be used in Homebrew/homebrew-core. Actions in this workflow must be: within a repository owned by Homebrew, created by GitHub, or matching the following: Vampire/setup-wsl@, codecov/codecov-action@, google-github-actions/setup-gcloud@, peter-evans/, reitermarkus/, ruby/setup-ruby@, dessant/lock-threads@, google-github-actions/auth@, google-github-actions/get-gke-credentials@, algolia/algoliasearch-crawler-github-actions@.

[woodruffw]

Yeah, looks like we'll need to revert here. That error message is funny (since the PR is in the Homebrew org); I'll triage with the GH folks in charge of the beta.

[p-linnane]

@MikeMcQuaid altered some settings, so we are trying again before reverting.

[p-linnane]

We've confirmed the failures are resolved now with the settings changes.

[woodruffw]

Reverted with #481. Post-mortem:

1. post-build originally began failing here: https://github.com/Homebrew/homebrew-core/actions/runs/7586024251/job/20663392584?pr=160418, which turned out to be an allowlist issue.
2. Once the action was allowlisted, it began failing with a different error: https://github.com/Homebrew/homebrew-core/actions/runs/7586024251/job/20663602342#step:4:377
3. That error indicates that the workflow doesn't have sufficient permissions, specifically id-token: write. Additionally, the workflow will need contents: write and packages: write.
4. The reason the workflow doesn't have sufficient permissions is twofold:
   1. They aren't explicitly configured in the permissions
   2. The workflow ran from a PR on a fork, which won't have access to id-token: write even when explicitly configured

Given the above, we need the following:

1. To ensure the workflow(s) that call this action have the appropriate permissions settings;
2. To ensure this action only gets calls from homebrew-core itself (which will also ensure that the identity in the provenance is the one we expect)

(2) seems like a significant limitation, since the Homebrew CI/CD currently assumes that arbitrary fork PRs can perform bottle builds. So we may need to think about a different "home" action for this provenance step.

CC @josephsweeney

[Bo98]

Would it make sense to be on bottle publish instead? That's the point where we take the bottles from the fork build and move them into a non-fork workflow that uploads them to GitHub Packages. That stage only runs after being approved by a maintainer so we also wouldn't be signing arbitrary stuff that anyone can throw at us.

[josephsweeney]

So I think after what we've learned we need to do the following:

1. I think the build provenance action should still be called post-build, but it should be put behind a new input boolean that defaults to false so that you have to explicitly pass it in.
2. Edit the test.yml to have the permissions as that is the workflow that ultimately calls post-build during publishing of bottles.
3. Add another input to test.yml either in that workflow or in the Ruby code in pr-pull that calls the workflow, that gets passed along to the post-build action telling it to sign the built bottles.

That should only sign the bottles that have been approved. The only hiccup is that expands the permissions of all test runs, which seems like a bad idea. If permissions can be adjusted for individual workflow runs, then that should mitigate that issue though, I need to look into that.

[Bo98]

Edit the test.yml to have the permissions as that is the workflow that ultimately calls post-build during publishing of bottles.

That flag there is just to say which workflow to download the artifacts from, since that information is necessary to pass to the GitHub API: https://github.com/Homebrew/brew/blob/c63723bd7d13668f39c9c68102218be88904b797/Library/Homebrew/utils/github.rb#L290-L292. tests.yml is not run during publishing.

I guess the complex bit there is pr-pull currently downloads the artifacts and uploads them immediately but we want to insert a step in the middle of that. I guess the solution there is passing --no-upload and then do a separate brew pr-publish: https://github.com/Homebrew/brew/blob/c63723bd7d13668f39c9c68102218be88904b797/Library/Homebrew/dev-cmd/pr-pull.rb#L490-L502

[josephsweeney]

Ah, I had the steps wrong then. I hadn't realized that the bottles are actually usually build when testing the PR's, then those artifacts are actually the bottles uploaded to GitHub packages, I thought they were built fresh.

Given this, the other option is to go back to a previous plan I had (to which I thought this was a simpler shortcut 😅) where we make a new standalone workflow that lives in homebrew-core. This workflow downloads the same artifact that will be uploaded and generates the build provenance. I can then kick this off from pr-upload (somewhere in this method probably).

Let me know if you see a problem with that approach though. As you can see my mental model of bottling is clearly immature.

[Bo98]

What you describe seems ok in principle. Though downloading twice would be nice to avoid given how big some bottles are. Feel free to ping me in a draft PR and I can have a closer look.

[Bo98]

If this helps: here's a general idea of what I was thinking:

diff --git a/.github/workflows/publish-commit-bottles.yml b/.github/workflows/publish-commit-bottles.yml
index 436bb135e8e..705400bfa30 100644
--- a/.github/workflows/publish-commit-bottles.yml
+++ b/.github/workflows/publish-commit-bottles.yml
@@ -315,11 +315,19 @@ jobs:
             --no-cherry-pick \
             --workflows=tests.yml \
             --committer="$BREWTESTBOT_NAME_EMAIL" \
+            --no-upload \
-            --root-url="https://ghcr.io/v2/homebrew/core" \
-            ${{inputs.warn_on_upload_failure && '--warn-on-upload-failure' || ''}} \
             ${{inputs.message && '--message="$INPUT_MESSAGE"' || ''}} \
             "$PR"

+          # additional bottle processing goes here - the bottles should be available in the working directory (hopefully).
+          # you should be able to easily split this into separate steps so that you can insert the provenance action here.
+
+          brew pr-upload \
+            --debug \
+            --committer="$BREWTESTBOT_NAME_EMAIL" \
+            --root-url="https://ghcr.io/v2/homebrew/core" \
+            ${{inputs.warn_on_upload_failure && '--warn-on-upload-failure' || ''}}
+
           echo "head_sha=$(git rev-parse HEAD)" >> "$GITHUB_OUTPUT"

       - name: Push commits

Assuming it needs to happen before uploading to GitHub Packages (probably a good idea so that we don't upload anything if we can't sign it for some reason).

[josephsweeney]

It seems like no matter what, we need an extra download, as GH actions can only be called from a workflow (unless I am wrong here).

That was originally why I decided to not try to call it from the ruby code, because you need to get the tarball into the workflow somehow (downloading the artifact seems to be the only way).

If we add a call there, it would just be running a workflow that calls the generate-build-provenance action so it would be necessary to download the artifact to that workflow regardless.

[Bo98]

Does it need to be a separate workflow? Can't the generate-build-provenance be in-place there? The publish workflow already has elevated write privileges which you can add id-token etc too.

[josephsweeney]

Yes, you are completely right. I had thought that diff was in the pr-upload code, not the publish workflow. I will make a draft PR and share. Thank you for your help!

[carlocab]

+          # additional bottle processing goes here - the bottles should be available in the working directory (hopefully).

Unfortunately not 😬:

https://github.com/Homebrew/brew/blob/13c4d0ac11fbe4af23339183ef7ae156b8a10b6d/Library/Homebrew/dev-cmd/pr-pull.rb#L447-L448

It is probably possible to call the generate-build-provenance workflow from within pr-pull itself after the guard for --no-upload. We assume the caller has sufficient permissions there anyway.

Or, actually, probably a better place is within pr-upload itself.

[Bo98]

https://github.com/Homebrew/brew/blob/13c4d0ac11fbe4af23339183ef7ae156b8a10b6d/Library/Homebrew/dev-cmd/pr-pull.rb#L447-L448

We can probably have a flag to control that so that it retains in the working directory