Expose PEP 740 attestations functionality

[woodruffw]

~~WIP, still experimenting here. Not ready for review 🙂~~

This adds PEP 740 attestation generation to the workflow: when the Trusted Publishing flow is used, this will generate a publish attestation for each distribution being uploaded. These generated attestations are then fed into twine, which newly supports them via --attestations.

Breakdown:

• [x] Generate attestations for each dist
• [x] Actually hook up twine --attestations
• [x] Documentation

See: https://github.com/pypi/warehouse/issues/15871

[woodruffw]

I've confirmed that the basic version of this works as expected (attestations is currently ignored by PyPI and TestPyPI): https://github.com/woodruffw-experiments/gha-pep740-experiments/actions/runs/9008318313/job/24750093954

[webknjaz]

@woodruffw I just bumped Twine FYI. And pre-commit is fine now. Rebasing should get the blockers out of the way.

[woodruffw]

@woodruffw I just bumped Twine FYI. And pre-commit is fine now. Rebasing should get the blockers out of the way.

Good timing, so did @facutuesca 😅

[facutuesca]

I'm squashing and rebasing this branch now

[facutuesca]

@woodruffw @webknjaz The remaining lint failure is due to an error message string (_TOKEN_RETRIEVAL_FAILED_MESSAGE) being duplicated in attestations.py and oidc-exchange.py. Would you prefer de-duplicating it by moving it into a new common.py file, or another approach?

[woodruffw]

Would you prefer de-duplicating it by moving it into a new common.py file, or another approach?

That's what I was thinking originally, although common.py won't be on sys.path so importing it won't be straightforward 🙁

I think the "clean" thing to do here would be to turn the Python files here into a project structure that gets installed as part of the Docker image's build. But that's a little heavyweight, so @webknjaz may have another idea 🙂

[webknjaz]

I'm leaning towards just being available on $PYTHONPATH, not pip installed. The way I did it @ https://github.com/re-actors/alls-green/blob/223e4bb/action.yml#L50-L53. However, such a restructuring should be separate and would probably include a src/pypi_publish/__main__.py entrypoint invoking the current main script through subprocess as the first step. I don't want a huge PR since big ones tend to cause long reviews and merge delays. Another thing to do would be to start to use our own managed virtualenv in the container rather than installing the deps globally into the shared userspace.

[webknjaz]

@woodruffw hey, it looks like GitHub rolled out their own attestations in beta: https://github.com/actions/attest-build-provenance / https://github.com/pypa/gh-action-pypi-publish/attestations / https://github.com/orgs/community/discussions/122028. I wonder if we could somehow integrate with that...

And it seems like there's even a new privilege attestations: write introduced for that on the platform: https://docs.github.com/en/actions/security-guides/using-artifact-attestations-to-establish-provenance-for-builds / https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#jobsjob_idpermissions.

[woodruffw]

(Sorry, was on PTO -- catching up on mentions now)

@woodruffw hey, it looks like GitHub rolled out their own attestations in beta: https://github.com/actions/attest-build-provenance / https://github.com/pypa/gh-action-pypi-publish/attestations / https://github.com/orgs/community/discussions/122028. I wonder if we could somehow integrate with that...

Yep, I've been thinking about how to make integrate the two -- the last comment on the PEP discussion thread suggests an approach that would allow GitHub-generated attestations to be compatible with this PEP.

TL;DR: I think our options here are:

1. Move forward with the current "version 1" specified in the PEP. This would mean no further changes are required on the PEP side. However, it would also mean that "version 1" isn't compatible with the default attestation format generated by GitHub, and that we would have to create a "version 2" that is compatible.
2. Edit the PEP in its current form and make it compatible with GitHub's attestation format. This would be a small change to the PEP overall, but has knock-on consequences (e.g. conceptually unbounded JSON payloads being signed over).

[woodruffw]

Just to update here: I've begun updating PEP 740 to be compatible with what GitHub's artifact attestations feature is doing, and it looks like it'll be pretty smooth.

That then leaves the question of how to approach attestation generation here 🙂:

1. Go forward with what's currently in this PR (a small script that uses the id-token permission + pypi_attestation_models to manually generate the attestation)
2. Move to a more discrete/black-boxed approach, e.g. something like this:

steps:
  - name: attest
    # does not exist yet!
    uses: pypa/gh-action-pypi-attest
    with:
      # this would be the default; just for illustration
      packages-dir: dist/

  - name: publish
    uses: pypa/gh-action-pypi-publish@release/v1

The benefit to approach (2) is that it's more devolved and can be built up on top of actions/attest@v1. OTOH it requires users to add another workflow step and get the order right, versus doing it implicitly in this action. Thoughts @webknjaz?

Edit: for context, the reason we can't use actions/attest-build-provenance or actions/attest directly is because (1) we have our own custom predicate, and (2) our attestation format requires a transformation step from the output bundle produced by those actions, so we need to wrap them in a "composite" or similar action to get everything into the shape PyPI expects.

[ofek]

Awesome stuff! I would much prefer option 1, we should prioritize UX.

[sethmlarson]

I agree with @ofek that ergonomics is the most important part here, requiring users add another action step means we will see delayed rollout of the feature compared to adding it on to the existing workflow.

[woodruffw]

SG, I'll keep going with the current approach then 🙂

[woodruffw]

This is good for an initial review! To summarize:

1. By default, nothing changes.
2. If the user sets attestations: true and is in a Trusted Publishing flow, this workflow will now transparently generate a "PyPI Publish" attestation for them, for each distribution being uploaded.
3. This attestation will be transparently stabled to each distribution, thanks to twine --attestations.

[sethmlarson]

@woodruffw Just double checking long-term strategy, is the attestations: true setting only there because this is an experimental feature and the goal is that one day a new version of the workflow will automatically do this without the attestations: true setting?

[woodruffw]

Just double checking long-term strategy, is the attestations: true setting only there because this is an experimental feature and the goal is that one day a new version of the workflow will automatically do this without the attestations: true setting?

Yes, exactly -- my thinking is that it's already non-default due to the tail of people using older versions of this action, so having it be (temporarily) opt-in makes experimenting easier while giving us the ability to flip the switch once everything is stable.

[webknjaz]

Sorry I didn't react earlier. I also think that UX is important. Not sure how well it plays with uploading attestations to GitHub, though.

Can we attempt relying on the attestations: write privilege instead of the action input? Can we feature-detect that it's enabled and use that as a flag? It feels like better ergonomics + potentially better integration with GH IIUC.

Another thought: why do we need to make attestations opt-in even? If it's supported in the Twine version and PyPI side is a no-op (still?), what's the harm in doing it unconditionally whenever we have OIDC privilege? If you're worried about crashes in our own code, we could just stick overly generic exception handling for the experimentation period.

[webknjaz]

OTOH it requires users to add another workflow step and get the order right, versus doing it implicitly in this action. Thoughts @webknjaz?

If we were to do this, I'd probably put it into a separate job, limiting the attestations: write privilege to only that, like it's done in the PyPUG guide. But it seems we're drifting away from this idea for the time being.

[webknjaz]

(edited to link the correct PR)

so we need to wrap them in a "composite" or similar action to get everything into the shape PyPI expects.

FYI, #230 works on wrapping this docker action into composite. So theoretically, we could stick another step into it once it's complete.

[woodruffw]

Can we attempt relying on the attestations: write privilege instead of the action input? Can we feature-detect that it's enabled and use that as a flag? It feels like better ergonomics + potentially better integration with GH IIUC.

That's nicer UX-wise, although as-implemented this feature doesn't actually require the attestations: write permission (since we're not touching any of GitHub's attestation features; we're doing the attestation 100% ourselves using the OIDC credential and sigstore-python).

Another thought: why do we need to make attestations opt-in even? If it's supported in the Twine version and PyPI side is a no-op (still?), what's the harm in doing it unconditionally whenever we have OIDC privilege? If you're worried about crashes in our own code, we could just stick overly generic exception handling for the experimentation period.

This is a fair point 🙂 -- my only argument for keeping it opt-in is that we don't want to flood PyPI (and TestPyPI) with a bunch of attestations while still in testing, especially if me or others make a mistake and cause uploads to generally fail when attestations are stapled to them.

(I have no strong opinion here, though.)

[webknjaz]

This is a fair point 🙂 -- my only argument for keeping it opt-in is that we don't want to flood PyPI (and TestPyPI) with a bunch of attestations while still in testing, especially if me or others make a mistake and cause uploads to generally fail when attestations are stapled to them.

(I have no strong opinion here, though.)

We did this with OIDC, though, didn't we? Nothing disastrous happened. Do we have stats on the trusted publishing adoption / upload frequency these days? Perhaps, we could estimate the impact and make an informed decision / educated guess.

[woodruffw]

To take a step back:

• The UX with the current approach is "nice", since the user's permissions don't change and we can transparently generate an attestation for them.
• But our attestations with the current approach aren't linked to GitHub's attestations feature, since we're generating them ourselves.
• To link them to GitHub's attestations feature, we could add attestations: write (and probably sniff for that) and switch to a separate action that gets used as a composite step. That would reduce the amount of new Python code necessary here, with the tradeoff being that it needs to block until the composite refactor is done.

TL;DR: we can maintain the "nice" UX with a separate action dedicated to the attestation, but only if this workflow becomes a composite first. Otherwise, the path of least resistance is probably to keep it as a bespoke attestation generator, with the downside that attestations generated this way won't show up on the GH repo's attestations page.

---

We did this with OIDC, though, didn't we? Nothing disastrous happened. Do we have stats on the trusted publishing adoption / upload frequency these days? Perhaps, we could estimate the impact and make an informed decision / educated guess.

That's true, although the error mode is slightly different here (people opted into trusted publishing by configuring it, which could then fail with a well defined error message, versus a normal TP workflow suddenly failing because I did a bad merge on the attestations feature on PyPI).

But I take your point and I'm mostly speculating 🙂. If you feel that the setting should be flipped or removed, I'm happy to do that.

In terms of stats, here's the latest I have:

So 13K projects have TP registered, 10K have used it, for 251K total dists uploaded with TP.

[sethmlarson]

That's true, although the error mode is slightly different here (people opted into trusted publishing by configuring it, which could then fail with a well defined error message, versus a normal TP workflow suddenly failing because I did a bad merge on the attestations feature on PyPI).

I'm definitely -1 on turning this feature on before we have some time to experiment, Trusted Publishers has extremely positive reception from the community (evidenced by the 13K projects w/ it configured without much of any "push" to do so). I would like to avoid having that soured by causing issues to projects during a pretty precious time, when you're trying to create a release.

I definitely do want to turn the feature on by default once we've had time to experiment ourselves, though! Just want to be careful with public perception of the feature.

[webknjaz]

I agree with both of you. Just wanted to hear your thoughts on these aspects.

Regarding the thing with our Sigstore instance vs. GitHub's: I feel like perhaps we could steer the end-users to use both. This action could use the regular sigstore server with the OIDC privilege already present. And that would be the attestation of what's uploaded — this is signing artifacts retrieved from somewhere outside the job already. However, the users could use GitHub's attestation mechanism in a separate job, perhaps. Though, this is probably overengineering. I feel some dissonance with our recommendation of not slapping OIDC privilege on the build jobs because of the additional attack surface, but OTOH, disconnected signing is also something that might lose its value in this scenario. What do y'all think on this topic?

[woodruffw]

Regarding the thing with our Sigstore instance vs. GitHub's: I feel like perhaps we could steer the end-users to use both. This action could use the regular sigstore server with the OIDC privilege already present. And that would be the attestation of what's uploaded — this is signing artifacts retrieved from somewhere outside the job already. However, the users could use GitHub's attestation mechanism in a separate job, perhaps.

Just to clarify: both are using the same Sigstore instance: they're both using the "public good" instance of Sigstore, which is hosted by the Sigstore project itself 🙂

The only real distinction between the two is how they're integrated into GitHub: GitHub's attestation feature has access to private APIs that add things to the <repo>/attestations view, while sigstore-python can generate the same attestations but not add them to that view.

But yes, I agree about using both: I think this action should generate "publish" attestations directly (which is what this PR does), and also accept "build" attestations generated by any preceding step (which can be done using GitHub's official workflows, or any other way). As currently implemented, this should work as-is: any previous attestations will be passed through to twine, as long as they match the expected filename pattern of *.*.attestation.

[woodruffw]

@webknjaz This could use another round of review, when you have time!

(Please let me know if there's anything I can do to make the changes more tractable here -- I'm happy to try anything that'll make this easier to review 🙂)

[gaborbernat]

I'm happy to give this a try once I return to Los Angeles on the 22nd.

[gaborbernat]

I am happy to try this out with some of my projects, should that help 👍

[woodruffw]

I am happy to try this out with some of my projects, should that help 👍

Feel free! It should be as easy as replacing the stable v1 ref with my fork + branch and enabling the attestations setting on the action.

(You may want to start with just TestPyPI, to avoid spamming the admins with alerts on production.)

[gaborbernat]

None of my projects officially use the test environment.😂