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

Expose PEP 740 attestations functionality #236

Merged
merged 37 commits into from
Sep 1, 2024
Merged
Show file tree
Hide file tree
Changes from 34 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
b526ff8
requirements: Add initial support for uploading PEP 740 attestations
woodruffw May 2, 2024
f267787
Misc lint fixes
facutuesca May 16, 2024
1571a0d
bump pypi_attestation_models, update usage
woodruffw Jun 11, 2024
27500cf
attestations: single quotes
woodruffw Jun 11, 2024
e9c72dd
attestations: simplify err
woodruffw Jun 11, 2024
3166978
Merge remote-tracking branch 'upstream/unstable/v1' into ww/attestations
woodruffw Jun 11, 2024
e7bd6ea
README: add a link
woodruffw Jun 11, 2024
5aa7e41
runtime: constrain pypi-attestation-models with a range
woodruffw Jun 18, 2024
4bc4ced
Merge branch 'unstable/v1' into ww/attestations
woodruffw Jun 18, 2024
0e2b9c9
runtime: bump range for pypi-attestation-models
woodruffw Jun 20, 2024
242d7e9
requirements: refreeze
woodruffw Jun 21, 2024
aa69903
Update requirements/runtime.in
woodruffw Jun 24, 2024
6b4d371
attestations: pre-validate dists as files
woodruffw Jun 24, 2024
16aa3a2
README: relocate PEP 740 info
woodruffw Jun 24, 2024
6dbccb5
README: PEP 740 -> "digital attestations"
woodruffw Jun 24, 2024
16b5dc1
README: explain that digital attestations require TP
woodruffw Jun 24, 2024
251402e
attestations: fix pylint
woodruffw Jun 24, 2024
1e91a3b
twine-upload: debug -> notice, rm PEP ref
woodruffw Jun 24, 2024
835d65d
attestations: debug dists before signing
woodruffw Jun 24, 2024
95be6b9
twine-upload: factor out TRUSTED_PUBLISHING
woodruffw Jun 24, 2024
176c905
pypi_attestation_models -> pypi_attestations
woodruffw Jun 28, 2024
9bac976
Merge remote-tracking branch 'upstream/unstable/v1' into ww/attestations
woodruffw Jul 9, 2024
6a808bf
runtime: bump constraints
woodruffw Jul 9, 2024
1bb6510
requirements: bump pypi-attestations
woodruffw Jul 10, 2024
8c640e3
bump to pypi-attestations==0.0.9
woodruffw Jul 17, 2024
e6556ab
attestations: use __main__ scope
woodruffw Jul 17, 2024
8094cdf
attestations: add main
woodruffw Jul 17, 2024
57dba07
attestations: please the linter
woodruffw Jul 17, 2024
af78f7a
README: emphasize beta
woodruffw Jul 22, 2024
bcc935f
twine-upload: emphasize attestations is a setting
woodruffw Jul 22, 2024
66f02b6
twine-upload: setting -> input
woodruffw Jul 22, 2024
28806ba
requirements: bump pypi-attestations, sigstore
woodruffw Jul 31, 2024
fed8784
requirements: bump sigstore, pypi-attestations
woodruffw Aug 20, 2024
61ffce1
Update attestations.py
woodruffw Aug 21, 2024
e1b63c3
Apply suggestions from code review
woodruffw Aug 21, 2024
15d9377
attestations: use Path.resolve(), break out dist collection
woodruffw Aug 21, 2024
473ca48
attestations: use exists() instead of is_file()
woodruffw Aug 21, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ COPY LICENSE.md .
COPY twine-upload.sh .
COPY print-hash.py .
COPY oidc-exchange.py .
COPY attestations.py .

RUN chmod +x twine-upload.sh
ENTRYPOINT ["/app/twine-upload.sh"]
29 changes: 29 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,31 @@ filter to the job:
if: github.event_name == 'push' && startsWith(github.ref, 'refs/tags')
```

### Generating and uploading attestations
woodruffw marked this conversation as resolved.
Show resolved Hide resolved

> [!IMPORTANT]
> Support for generating and uploading [digital attestations] is currently
> experimental and limited only to Trusted Publishing flows using PyPI or TestPyPI.
> Support for this feature is not yet stable; the settings and behavior described
> below may change without prior notice.

> [!NOTE]
> Generating and uploading digital attestations currently requires
> authentication with a [trusted publisher].

You can generate signed [digital attestations] for all the distribution files and
upload them all together by enabling the `attestations` setting:

```yml
with:
attestations: true
```

This will use [Sigstore] to create attestation
objects for each distribution package, signing them with the identity provided
by the GitHub's OIDC token associated with the current workflow. This means
both the trusted publishing authentication and the attestations are tied to the
same identity.

## Non-goals

Expand Down Expand Up @@ -287,3 +312,7 @@ https://github.com/vshymanskyy/StandWithUkraine/blob/main/docs/README.md
[configured on PyPI]: https://docs.pypi.org/trusted-publishers/adding-a-publisher/

[how to specify username and password]: #specifying-a-different-username

[digital attestations]: https://peps.python.org/pep-0740/
[Sigstore]: https://www.sigstore.dev/
[trusted publisher]: #trusted-publishing
8 changes: 8 additions & 0 deletions action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,13 @@ inputs:
Use `print-hash` instead.
required: false
default: 'false'
attestations:
description: >-
[EXPERIMENTAL]
Enable experimental support for PEP 740 attestations.
Only works with PyPI and TestPyPI via Trusted Publishing.
required: false
default: 'false'
branding:
color: yellow
icon: upload-cloud
Expand All @@ -95,3 +102,4 @@ runs:
- ${{ inputs.skip-existing }}
- ${{ inputs.verbose }}
- ${{ inputs.print-hash }}
- ${{ inputs.attestations }}
108 changes: 108 additions & 0 deletions attestations.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
import logging
import os
import sys
from pathlib import Path
from typing import NoReturn

from pypi_attestations import Attestation, Distribution
from sigstore.oidc import IdentityError, IdentityToken, detect_credential
from sigstore.sign import Signer, SigningContext

# Be very verbose.
sigstore_logger = logging.getLogger('sigstore')
sigstore_logger.setLevel(logging.DEBUG)
sigstore_logger.addHandler(logging.StreamHandler())

_GITHUB_STEP_SUMMARY = Path(os.getenv('GITHUB_STEP_SUMMARY'))

# The top-level error message that gets rendered.
# This message wraps one of the other templates/messages defined below.
_ERROR_SUMMARY_MESSAGE = """
Attestation generation failure:

{message}

You're seeing this because the action attempted to generated PEP 740
attestations for its inputs, but failed to do so.
"""

# Rendered if OIDC identity token retrieval fails for any reason.
_TOKEN_RETRIEVAL_FAILED_MESSAGE = """
OpenID Connect token retrieval failed: {identity_error}

This failure occurred after a successful Trusted Publishing Flow,
suggesting a transient error.
""" # noqa: S105; not a password


def die(msg: str) -> NoReturn:
with _GITHUB_STEP_SUMMARY.open('a', encoding='utf-8') as io:
print(_ERROR_SUMMARY_MESSAGE.format(message=msg), file=io)

# HACK: GitHub Actions' annotations don't work across multiple lines naively;
# translating `\n` into `%0A` (i.e., HTML percent-encoding) is known to work.
# See: https://github.com/actions/toolkit/issues/193
msg = msg.replace('\n', '%0A')
print(f'::error::Attestation generation failure: {msg}', file=sys.stderr)
sys.exit(1)


def debug(msg: str):
print(f'::debug::{msg}', file=sys.stderr)


def attest_dist(dist_path: Path, signer: Signer) -> None:
# We are the publishing step, so there should be no pre-existing publish
# attestation. The presence of one indicates user confusion.
attestation_path = Path(f'{dist_path}.publish.attestation')
if attestation_path.is_file():
woodruffw marked this conversation as resolved.
Show resolved Hide resolved
die(f'{dist_path} already has a publish attestation: {attestation_path}')

dist = Distribution.from_file(dist_path)
attestation = Attestation.sign(signer, dist)

attestation_path.write_text(attestation.model_dump_json(), encoding='utf-8')
debug(f'saved publish attestation: {dist_path=} {attestation_path=}')


def get_identity_token() -> IdentityToken:
# Will raise `sigstore.oidc.IdentityError` if it fails to get the token
# from the environment or if the token is malformed.
# NOTE: audience is always sigstore.
oidc_token = detect_credential()
return IdentityToken(oidc_token)


def main() -> None:
packages_dir = Path(sys.argv[1])

try:
identity = get_identity_token()
except IdentityError as identity_error:
# NOTE: We only perform attestations in trusted publishing flows, so we
# don't need to re-check for the "PR from fork" error mode, only
# generic token retrieval errors. We also render a simpler error,
# since permissions can't be to blame at this stage.
die(_TOKEN_RETRIEVAL_FAILED_MESSAGE.format(identity_error=identity_error))

# Collect all sdists and wheels.
dist_paths = [sdist.absolute() for sdist in packages_dir.glob('*.tar.gz')]
dist_paths.extend(whl.absolute() for whl in packages_dir.glob('*.whl'))
woodruffw marked this conversation as resolved.
Show resolved Hide resolved

# Make sure everything that looks like a dist actually is one.
# We do this up-front to prevent partial signing.
if (invalid_dists := [_path for _path in dist_paths if _path.is_file()]):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is where the bug snuck in that needed fixing by 0ab0b79.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤦 thanks for fixing that.

invalid_dist_list = ', '.join(map(str, invalid_dists))
die(
'The following paths look like distributions but '
f'are not actually files: {invalid_dist_list}',
)

with SigningContext.production().signer(identity, cache=True) as s:
debug(f'attesting to dists: {dist_paths}')
for dist_path in dist_paths:
attest_dist(dist_path, s)


if __name__ == '__main__':
main()
7 changes: 6 additions & 1 deletion requirements/runtime.in
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
twine

# NOTE: Used to detect an ambient OIDC credential for OIDC publishing.
# NOTE: Used to detect an ambient OIDC credential for OIDC publishing,
# as well as PEP 740 attestations.
woodruffw marked this conversation as resolved.
Show resolved Hide resolved
id ~= 1.0

# NOTE: This is pulled in transitively through `twine`, but we also declare
# NOTE: it explicitly here because `oidc-exchange.py` uses it.
# Ref: https://github.com/di/id
requests

# NOTE: Used to generate attestations.
pypi-attestations ~= 0.0.11
sigstore ~= 3.2.0
81 changes: 75 additions & 6 deletions requirements/runtime.txt
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,41 @@
#
annotated-types==0.6.0
# via pydantic
betterproto==2.0.0b6
# via sigstore-protobuf-specs
certifi==2024.2.2
# via requests
cffi==1.16.0
# via cryptography
charset-normalizer==3.3.2
# via requests
cryptography==42.0.7
# via
# pyopenssl
# pypi-attestations
# sigstore
dnspython==2.6.1
# via email-validator
docutils==0.21.2
# via readme-renderer
email-validator==2.1.1
# via pydantic
grpclib==0.4.7
# via betterproto
h2==4.1.0
# via grpclib
hpack==4.0.0
# via h2
hyperframe==6.0.1
# via h2
id==1.4.0
# via -r runtime.in
# via
# -r runtime.in
# sigstore
idna==3.7
# via requests
# via
# email-validator
# requests
importlib-metadata==7.1.0
# via twine
jaraco-classes==3.4.0
Expand All @@ -34,33 +59,77 @@ more-itertools==10.2.0
# via
# jaraco-classes
# jaraco-functools
multidict==6.0.5
# via grpclib
nh3==0.2.17
# via readme-renderer
packaging==24.1
# via pypi-attestations
pkginfo==1.10.0
# via twine
platformdirs==4.2.2
# via sigstore
pyasn1==0.6.0
# via sigstore
pycparser==2.22
# via cffi
pydantic==2.7.1
# via id
# via
# id
# pypi-attestations
# sigstore
# sigstore-rekor-types
pydantic-core==2.18.2
# via pydantic
pygments==2.18.0
# via
# readme-renderer
# rich
pyjwt==2.8.0
# via sigstore
pyopenssl==24.1.0
# via sigstore
pypi-attestations==0.0.11
# via -r runtime.in
python-dateutil==2.9.0.post0
# via betterproto
readme-renderer==43.0
# via twine
requests==2.32.0
requests==2.32.3
# via
# -r runtime.in
# id
# requests-toolbelt
# sigstore
# tuf
# twine
requests-toolbelt==1.0.0
# via twine
rfc3986==2.0.0
# via twine
rfc8785==0.1.2
# via sigstore
rich==13.7.1
# via twine
twine==5.1.0
# via
# sigstore
# twine
securesystemslib==1.0.0
# via tuf
sigstore==3.2.0
# via
# -r runtime.in
# pypi-attestations
sigstore-protobuf-specs==0.3.2
# via
# pypi-attestations
# sigstore
sigstore-rekor-types==0.0.13
# via sigstore
six==1.16.0
# via python-dateutil
tuf==5.0.0
# via sigstore
twine==5.1.1
# via -r runtime.in
typing-extensions==4.11.0
# via
Expand Down
42 changes: 41 additions & 1 deletion twine-upload.sh
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ INPUT_PACKAGES_DIR="$(get-normalized-input 'packages-dir')"
INPUT_VERIFY_METADATA="$(get-normalized-input 'verify-metadata')"
INPUT_SKIP_EXISTING="$(get-normalized-input 'skip-existing')"
INPUT_PRINT_HASH="$(get-normalized-input 'print-hash')"
INPUT_ATTESTATIONS="$(get-normalized-input 'attestations')"

PASSWORD_DEPRECATION_NUDGE="::error title=Password-based uploads disabled::\
As of 2024, PyPI requires all users to enable Two-Factor \
Expand All @@ -53,7 +54,37 @@ environments like GitHub Actions without needing to use username/password \
combinations or API tokens to authenticate with PyPI. Read more: \
https://docs.pypi.org/trusted-publishers"

if [[ "${INPUT_USER}" == "__token__" && -z "${INPUT_PASSWORD}" ]] ; then
ATTESTATIONS_WITHOUT_TP_WARNING="::warning title=attestations input ignored::\
The workflow was run with the 'attestations: true' input, but an explicit \
password was also set, disabling Trusted Publishing. As a result, the \
attestations input is ignored."

ATTESTATIONS_WRONG_INDEX_WARNING="::warning title=attestations input ignored::\
The workflow was run with 'attestations: true' input, but the specified \
repository URL does not support PEP 740 attestations. As a result, the \
attestations input is ignored."

[[ "${INPUT_USER}" == "__token__" && -z "${INPUT_PASSWORD}" ]] \
&& TRUSTED_PUBLISHING=true || TRUSTED_PUBLISHING=false

if [[ "${INPUT_ATTESTATIONS}" != "false" ]] ; then
# Setting `attestations: true` without Trusted Publishing indicates
# user confusion, since attestations (currently) require it.
if ! "${TRUSTED_PUBLISHING}" ; then
echo "${ATTESTATIONS_WITHOUT_TP_WARNING}"
INPUT_ATTESTATIONS="false"
fi

# Setting `attestations: true` with an index other than PyPI or TestPyPI
# indicates user confusion, since attestations are not supported on other
# indices presently.
if [[ ! "${INPUT_REPOSITORY_URL}" =~ pypi\.org ]] ; then
echo "${ATTESTATIONS_WRONG_INDEX_WARNING}"
INPUT_ATTESTATIONS="false"
fi
Comment on lines +78 to +84
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this necessary given pypa/twine#1099?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not necessary, since twine will catch this case and produce an appropriate error. OTOH catching it here might make the error slightly more intelligible, since the twine error doesn't get propagated into a ::warning annotation like this one does.

This is similar to how the action pre-validates the dist directory, even though twine also validates it. But if you think it makes it makes things more confusing than necessary, I can drop this check 🙂

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's probably fine for this to catch it before twine and give a more tailored error message, I'm mostly just thinking about the long-term pain of maintaining/updating this list of supported indexes in multiple places, but this is fine for now.

fi

if "${TRUSTED_PUBLISHING}" ; then
# No password supplied by the user implies that we're in the OIDC flow;
# retrieve the OIDC credential and exchange it for a PyPI API token.
echo "::debug::Authenticating to ${INPUT_REPOSITORY_URL} via Trusted Publishing"
Expand Down Expand Up @@ -130,6 +161,15 @@ if [[ ${INPUT_VERBOSE,,} != "false" ]] ; then
TWINE_EXTRA_ARGS="--verbose $TWINE_EXTRA_ARGS"
fi

if [[ ${INPUT_ATTESTATIONS,,} != "false" ]] ; then
# NOTE: Intentionally placed after `twine check`, to prevent attestation
# generation on distributions with invalid metadata.
woodruffw marked this conversation as resolved.
Show resolved Hide resolved
echo "::notice::Generating and uploading digital attestations"
python /app/attestations.py "${INPUT_PACKAGES_DIR%%/}"

TWINE_EXTRA_ARGS="--attestations $TWINE_EXTRA_ARGS"
fi

if [[ ${INPUT_PRINT_HASH,,} != "false" || ${INPUT_VERBOSE,,} != "false" ]] ; then
python /app/print-hash.py ${INPUT_PACKAGES_DIR%%/}
fi
Expand Down
Loading