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

Attest build provenance of artifacts #122

Merged
merged 5 commits into from
May 13, 2024

Conversation

hugovk
Copy link
Contributor

@hugovk hugovk commented May 8, 2024

Relates to #105.

Attest using https://github.com/actions/attest-build-provenance which uses Sigstore.

See https://github.blog/2024-05-02-introducing-artifact-attestations-now-in-public-beta/ for more info.

I'm not really sure how to test this, or if you want to add this. Feel free to take over, or close :)


It's very easy to add it to a regular workflow, for example:

hugovk/tinytext@50f5520

It signs the files before uploading to PyPI.

Then you can download them from https://test.pypi.org/project/tinytext/3.6.1.dev93/#files and verify the owner or repo using the GitHub CLI:

gh attestation verify tinytext-3.6.1.dev94-py3-none-any.whl --owner hugovk
Loaded digest sha256:5933462a4b047f8d46cfd5dea3c27195ad569509bbfcb6d6e4a9cb2eed7329ad for file://tinytext-3.6.1.dev94-py3-none-any.whl
Loaded 1 attestation from GitHub API
✓ Verification succeeded!

sha256:5933462a4b047f8d46cfd5dea3c27195ad569509bbfcb6d6e4a9cb2eed7329ad was attested by:
REPO             PREDICATE_TYPE                  WORKFLOW
hugovk/tinytext  https://slsa.dev/provenance/v1  .github/workflows/deploy.yml@refs/heads/maingh attestation verify tinytext-3.6.1.dev94.tar.gz --repo hugovk/tinytext
Loaded digest sha256:9813cf6a34fa06f6a649c50c2a50cad04df070b52f68343c067ffb3a0bb19539 for file://tinytext-3.6.1.dev94.tar.gz
Loaded 1 attestation from GitHub API
✓ Verification succeeded!

sha256:9813cf6a34fa06f6a649c50c2a50cad04df070b52f68343c067ffb3a0bb19539 was attested by:
REPO             PREDICATE_TYPE                  WORKFLOW
hugovk/tinytext  https://slsa.dev/provenance/v1  .github/workflows/deploy.yml@refs/heads/main

@hynek
Copy link
Owner

hynek commented May 11, 2024

Looks almost too easy! ;) I’m also not sure how to test it… @sethmlarson any opinions on correctness and/or testability? 🤔

@sethmlarson
Copy link

Since you're uploading the artifacts to GitHub's artifact store having this signature information makes sense to me. I don't see any issues with the implementation, for testing you could run the action and then verify it with GitHub's CLI against the repository? Doing that once manually might be enough without needing to encode it into CI.

@hynek
Copy link
Owner

hynek commented May 11, 2024

Right, if both of you give it a 👍 I’m fine. I’ll try to land plus ship before PyCon!

@hugovk changelog would be nice. 🤓

@hugovk
Copy link
Contributor Author

hugovk commented May 11, 2024

Added!

@woodruffw
Copy link

One minor note: this is going to use GitHub's attestation format, which isn't what we're currently standardizing with PEP 740 😅.

Given how easy it is to use GitHub's feature, however, I'm tempted to adapt PEP 740 to make it compatible with their approach. But I'll need to get in front of a computer on Monday to see what that'll involve 🙂

(TL;DR PyPI won't accept attestations in this format under the current proposed PEP. But that isn't necessarily a blocker here, since these ones are only ending up in GitHub's store anyways.)

@hynek
Copy link
Owner

hynek commented May 11, 2024

Maybe we should just stress the fact that it’s GH’s store for now and maybe convene at PyCon NEXT WEEK? 😱

@sethmlarson
Copy link

sethmlarson commented May 11, 2024

One minor note: this is going to use GitHub's attestation format, which isn't what we're currently standardizing with PEP 740 😅.

@woodruffw This is what I was thinking too but because the artifacts aren't destined for PyPI (and if they are, publish provenance will happen in the upload action) maybe it's okay to only do GitHub's? Let's all chat at PyCon!

@woodruffw
Copy link

Sounds good!

@hynek
Copy link
Owner

hynek commented May 12, 2024

I think we could/should still ship it until then, right? Would it make sense to rename it to attest-build-provenance-github and call it a day?

@hugovk
Copy link
Contributor Author

hugovk commented May 12, 2024

Something like that sounds good.

I can make the change soon but I'll be in a big metal cylinder and won't be able to push for many hours, so please take over if you like :)

@hynek
Copy link
Owner

hynek commented May 12, 2024

I'll be in a big metal cylinder and won't be able to push for many hour

Safe travels, I'll be with you in thoughts. 😜

@hynek
Copy link
Owner

hynek commented May 13, 2024

Hmm, what does this mean: Error: Failed to get ID token: Error message: Unable to get ACTIONS_ID_TOKEN_REQUEST_URL env variable?

Does it need more setup (that should be documented) or did it catch the fact I'm trying to attest a package from a "wrong" repo?

@woodruffw
Copy link

woodruffw commented May 13, 2024

Does it need more setup (that should be documented) or did it catch the fact I'm trying to attest a package from a "wrong" repo?

I think it's the latter: GitHub Actions only grants the id-token: write permission on first-party workflow runs, so a PR from a fork doesn't have access to the identity credential needed for signing (even if explicitly approved).

(IMO the way it fails is pretty bad: ideally the workflow would fail on id-token: write being present, rather than continuing until the injected environment variable fails to exist.)

Concretely, I think this workflow step needs to be skipped on non-first-party PRs or made to run only on a specific branch push pattern (e.g. main).

Edit: for context, here's how I perform a skip in a similar repo:

if: (github.event_name != 'pull_request') || !github.event.pull_request.head.repo.fork

i.e.: "run this step if it's not a PR or it's a PR, but the PR is not from a fork."

@hynek
Copy link
Owner

hynek commented May 13, 2024

OK, cool. Otherwise this should be fit to ship? I plan some panicked and haphazard releases before (lolsob) PyCon, so that would be a nice demo I guess?

@woodruffw
Copy link

OK, cool. Otherwise this should be fit to ship? I plan some panicked and haphazard releases before (lolsob) PyCon, so that would be a nice demo I guess?

Yep, LGTM!

@hynek hynek merged commit 0f9f778 into hynek:main May 13, 2024
10 checks passed
@hugovk hugovk deleted the attest-build-provenance branch May 13, 2024 15:53
@hynek
Copy link
Owner

hynek commented May 13, 2024

Look at these beautiful attestations!

@webknjaz
Copy link
Contributor

I realized I didn't post in the related PR, so cross-linking my concerns on why this coupled implementation is dangerous: #105 (comment).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants