Implement trusted publishing #7548

konstin · 2024-09-19T13:26:07Z

Trusted publishing allows uploading to PyPI from GitHub actions without setting a (long-lived) secret token. Instead, you configure a GitHub Actions workflow as trusted publisher. With the id-token: write permission, GitHub Actions then allows us to obtain an OpenID Connect (OIDC) token, with which we can ask PyPI for a short lived upload token just for this session. The user experiences this as credentials-free upload. See https://docs.pypi.org/trusted-publishers/ for details and https://github.com/pypa/gh-action-pypi-publish for the reference implementation.

When we are in GitHub Actions and there are no explicit credentials, we try to obtain trusted publishing credentials. This can be controlled with --trusted-publishing (automatic, always, never)

The auth middleware gained a new option that allows us to selectively turn off request-cloning only (for uploads) or the entire middleware (for OIDC, which determines credentials through network requests), while still sharing the client once initialized.

Since we can't do testing offline, we upload astral-test-trusted-publishing in the test github action.

.github/workflows/ci.yml

docs/reference/settings.md

lucab · 2024-09-20T07:39:30Z

crates/uv-publish/src/trusted_publishing.rs

+    // If this fails, we can skip the audience request.
+    let oidc_token_request_token = env::var("ACTIONS_ID_TOKEN_REQUEST_TOKEN")?;
+
+    // When communicating with GitHub and PyPI, we don't want any custom ssl settings or retries, so


I suspect you'll actually encounter network failures to both GH and PyPI, action runners are very noisy-neighbor environments at time.
I think each individual HTTP call here would benefit from having its own timeout and retries.

As a datapoint, the last CI run on main is failing due to a network hang on a GH-to-GH fetch, during the setup of job environment itself.
Their client implements the following timeout-and-retry approach:

Download action repository 'actions/checkout@v4' (SHA:692973e3d937129bcbf40652eb9f2f61becf3332) Download action repository 'actions/download-artifact@v4' (SHA:fa0a91b85d4f404e444e00e005971372dc801d16) Warning: Failed to download action 'https://api.github.com/repos/actions/download-artifact/tarball/fa0a91b85d4f404e444e00e005971372dc801d16'. Error: The request was canceled due to the configured HttpClient.Timeout of 100 seconds elapsing. Warning: Back off 12.296 seconds before retry. Warning: Failed to download action 'https://api.github.com/repos/actions/download-artifact/tarball/fa0a91b85d4f404e444e00e005971372dc801d16'. Error: The request was canceled due to the configured HttpClient.Timeout of 100 seconds elapsing. Warning: Back off 11.758 seconds before retry. Error: Action 'https://api.github.com/repos/actions/download-artifact/tarball/fa0a91b85d4f404e444e00e005971372dc801d16' download has timed out. Error: The request was canceled due to the configured HttpClient.Timeout of 100 seconds elapsing.

That's good to know

lucab · 2024-09-20T07:41:44Z

crates/uv-publish/src/trusted_publishing.rs

+    // Tell GitHub Actions to mask the token in any console logs.
+    #[allow(clippy::print_stdout)]
+    {
+        // If we successfully obtained a token, we know we must be in GitHub Actions, so it's safe


This is probably fine now but may become a future hazard if more providers get added. It should be cheap to re-verify the sentinel flag in the env, or directly let the caller pass this information to here.

lucab · 2024-09-20T08:02:17Z

crates/uv-cli/src/lib.rs

@@ -4324,6 +4324,27 @@ pub struct PublishArgs {
    )]
    pub token: Option<String>,

+    /// Always use trusted publishing through GitHub Actions.
+    ///
+    /// By default, uv checks for trusted publishing when running in GitHub Actions, but ignores it


Self-note: this sets the default flow of the publishing logic in a way that doesn't require the user to opt-in into it, and I was wondering whether this a security surprise/hazard.
I convinced myself that this should be fine because, although this starts automatically using ambient capabilities that are provided (by default?) by GH and that the users may not be fully aware of, completing a proper authentication loop requires a setup with an explicit opt-in step by the user on the PyPi side.

charliermarsh · 2024-09-23T19:12:54Z

.github/workflows/ci.yml

@@ -943,6 +943,9 @@ jobs:
    env:
      # No dbus in GitHub Actions
      PYTHON_KEYRING_BACKEND: keyrings.alt.file.PlaintextKeyring
+    permissions:
+      # For trusted publishing
+      id-token: write


This is dubious, no?

It's from the docs:

https://docs.pypi.org/trusted-publishers/using-a-publisher/

It seems like it still may be dubious to run in our public CI, right?

We of course need to review all changes to the code called here, but the risk is otherwise the same as if someone makes a PR that adds id-token: write. The permission in testpypi itself is clearly scoped to allow only this package and only this environment.

crates/uv/src/commands/publish.rs

charliermarsh · 2024-09-23T19:14:51Z

crates/uv/src/commands/publish.rs

+    // * For the uploads themselves, we know we need an authorization header and we can't nor
+    //   shouldn't try cloning the request to make an unauthenticated request first, but we want
+    //   keyring integration. For trusted publishing, we use an OIDC auth routine without keyring
+    //   or other auth integration.


Yeah painful.

charliermarsh · 2024-09-23T19:16:14Z

crates/uv-publish/src/trusted_publishing.rs

+pub(crate) async fn get_token(
+    registry: &Url,
+    client: &ClientWithMiddleware,
+) -> Result<String, TrustedPublishingError> {


Can we create a wrapper type for the token?

The token we get gets used as the password later. We currently store the password as Option<String>, both in publish and the index api. Should we introduce a new Redacted type and use it for all password? (By using a newtype just for the token, we just have to turn it back to a string two functions down)

Still worth it to have a dedicated Token type from my perspective. Less String is always better.

charliermarsh · 2024-09-23T19:17:03Z

crates/uv-publish/src/trusted_publishing.rs

+    {
+        // If we successfully obtained a token, we know we must be in GitHub Actions, so it's safe
+        // to use GitHub Actions commands.
+        println!("::add-mask::{}", &publish_token);


Should we check the env var again to confirm that we're in GHA?

We can only enter the function if we're in github actions and the process can only succeed when we're in github actions (i.e. we can't obtain the token and execute this line without running in github actions), so it's save to assume we are in github actions.

Why is it a problem to check the env var again? Otherwise we risk printing the unmasked token in other environments.

charliermarsh · 2024-09-23T19:17:38Z

crates/uv-publish/src/trusted_publishing.rs

+) -> Result<String, TrustedPublishingError> {
+    // `pypa/gh-action-pypi-publish` uses `netloc` (RFC 1808), which is deprecated for authority
+    // (RFC 3986).
+    let audience_url = Url::parse(&format!("https://{}/_/oidc/audience", registry.authority()))?;


Nit: you could DRY up this URL builder which is also used below.

I had looked for this, but i couldn't find anything in the url crate that would allow me to build an url from an authority.

charliermarsh · 2024-09-23T19:17:50Z

crates/uv-publish/src/trusted_publishing.rs

+    oidc_token_request_token: &str,
+    client: &ClientWithMiddleware,
+) -> Result<String, TrustedPublishingError> {
+    let mut oidc_token_url = Url::parse(&env::var("ACTIONS_ID_TOKEN_REQUEST_URL")?)?;


Probably worth a dedicated error message if the env var is missing?

Where would that error message be displayed?

Like, here, instead of env::var("ACTIONS_ID_TOKEN_REQUEST_URL")?, was my thinking.

crates/uv-settings/src/settings.rs

charliermarsh · 2024-09-23T19:18:48Z

How was this tested? Can you include a test plan in the summary?

Update crates/uv-settings/src/Implement trusted publishing Co-authored-by: Charlie Marsh <[email protected]>

This MR contains the following updates: | Package | Update | Change | |---|---|---| | [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.4.15` -> `0.4.18` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>astral-sh/uv (astral-sh/uv)</summary> ### [`v0.4.18`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0418) [Compare Source](astral-sh/uv@0.4.17...0.4.18) ##### Enhancements - Allow multiple source entries for each package in `tool.uv.sources` ([#7745](astral-sh/uv#7745)) - Add `.gitignore` file to `uv build` output directory ([#7835](astral-sh/uv#7835)) - Disable jemalloc on FreeBSD ([#7780](astral-sh/uv#7780)) - Respect `PAGER` env var when paging in `uv help` command ([#5511](astral-sh/uv#5511)) - Support `uv run -m foo` to run a module ([#7754](astral-sh/uv#7754)) - Use a top-level output directory for `uv build` in workspaces ([#7813](astral-sh/uv#7813)) - Update `uv init --package` command to match project name ([#7670](astral-sh/uv#7670)) - Add a custom suggestion for `uv add dotenv` ([#7799](astral-sh/uv#7799)) - Add detailed errors for `tool.uv.sources` deserialization failures ([#7823](astral-sh/uv#7823)) - Improve error message copy for failed builds ([#7849](astral-sh/uv#7849)) - Use `serde-untagged` to improve some untagged enum error messages ([#7822](astral-sh/uv#7822)) - Use build failure hints for `dotenv` errors, rather than in `uv add` ([#7825](astral-sh/uv#7825)) ##### Configuration - Add `UV_NO_SYNC` environment variable ([#7752](astral-sh/uv#7752)) ##### Bug fixes - Accept `git+` prefix in `tool.uv.sources` ([#7847](astral-sh/uv#7847)) - Allow spaces in path requirements ([#7767](astral-sh/uv#7767)) - Avoid reusing cached downloaded binaries with `--no-binary` ([#7772](astral-sh/uv#7772)) - Correctly trims values during wheel WHEEL file parsing ([#7770](astral-sh/uv#7770)) - Fix `uv tree --invert` for platform dependencies ([#7808](astral-sh/uv#7808)) - Fix encoding mismatch between python child process and uv ([#7757](astral-sh/uv#7757)) - Reject self-dependencies in `uv add` ([#7766](astral-sh/uv#7766)) - Respect `tool.uv.environments` for legacy virtual workspace roots ([#7824](astral-sh/uv#7824)) - Retain empty extras on workspace members ([#7762](astral-sh/uv#7762)) - Use file stem when parsing cached wheel names ([#7773](astral-sh/uv#7773)) ##### Rust API - Make `FlatDistributions` public ([#7833](astral-sh/uv#7833)) ##### Documentation - Fix table of contents sizing ([#7751](astral-sh/uv#7751)) - GitLab Integration documentation ([#6857](astral-sh/uv#6857)) - Update documentation to setup-uv@v3 ([#7807](astral-sh/uv#7807)) - Use `uv publish` instead of twine in docs ([#7837](astral-sh/uv#7837)) - Fix typo in `projects.md` ([#7784](astral-sh/uv#7784)) ### [`v0.4.17`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0417) [Compare Source](astral-sh/uv@0.4.16...0.4.17) ##### Enhancements - Add `uv build --all` to build all packages in a workspace ([#7724](astral-sh/uv#7724)) - Add support for `uv init --script` ([#7565](astral-sh/uv#7565)) - Add support for upgrading build environment for installed tools (`uv tool upgrade --python`) ([#7605](astral-sh/uv#7605)) - Initialize a Git repository in `uv init` ([#5476](astral-sh/uv#5476)) - Respect `--quiet` flag in `uv build` ([#7674](astral-sh/uv#7674)) - Add context message before listing available tools in `uvx` ([#7641](astral-sh/uv#7641)) ##### Bug fixes - Don't create Python bytecode files during interpreter discovery ([#7707](astral-sh/uv#7707)) - Escape glob patterns in workspace member discovery ([#7709](astral-sh/uv#7709)) - Avoid prefetching source distributions with unbounded lower-bound ranges ([#7683](astral-sh/uv#7683)) ##### Documentation - Add `uv build` and `uv publish` to features overview ([#7716](astral-sh/uv#7716)) - Add documentation on cache versioning ([#7693](astral-sh/uv#7693)) - Spell out the names of the Docker images for easier copy-paste ([#7706](astral-sh/uv#7706)) - Document uv-with-Jupyter workflows ([#7625](astral-sh/uv#7625)) - Note that `uv lock --upgrade-package` retains locked versions ([#7694](astral-sh/uv#7694)) ### [`v0.4.16`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0416) [Compare Source](astral-sh/uv@0.4.15...0.4.16) ##### Enhancements - Add `uv publish` ([#7475](astral-sh/uv#7475)) - Add a `--project` argument to run a command from a project directory ([#7603](astral-sh/uv#7603)) - Display Python implementation when creating environments ([#7652](astral-sh/uv#7652)) - Implement trusted publishing for `uv publish` ([#7548](astral-sh/uv#7548)) - Respect lockfile preferences for `--with` requirements ([#7627](astral-sh/uv#7627)) - Unhide the `--directory` option ([#7653](astral-sh/uv#7653)) - Allow requesting free-threaded Python interpreters ([#7431](astral-sh/uv#7431)) - Show a dedicated PubGrub hint for `--unsafe-best-match` ([#7645](astral-sh/uv#7645)) - Add resolver error checking for conflicting distributions ([#7595](astral-sh/uv#7595)) ##### Bug fixes - Avoid adding double-newlines for CRLF ([#7640](astral-sh/uv#7640)) - Avoid retaining forks when `requires-python` range changes ([#7624](astral-sh/uv#7624)) - Determine if pre-release Python downloads should be allowed using the version specifiers ([#7638](astral-sh/uv#7638)) - Fix `link-mode=clone` for directories on Linux ([#7620](astral-sh/uv#7620)) - Improve Python executable name discovery when using alternative implementations ([#7649](astral-sh/uv#7649)) - Require opt-in to use alternative Python implementations ([#7650](astral-sh/uv#7650)) - Use the first pre-release discovered when only pre-release Python versions are available ([#7666](astral-sh/uv#7666)) ##### Documentation - Document environment variable that disables printing of virtual environment name in prompt ([#7648](astral-sh/uv#7648)) - Remove double whitespaces from the code ([#7623](astral-sh/uv#7623)) - Use anchorlinks rather than permalinks ([#7626](astral-sh/uv#7626)) ##### Preview features - Add build backend scaffolding ([#7662](astral-sh/uv#7662)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).

konstin force-pushed the konsti/publish2 branch from 6f83d3f to f72da4d Compare September 19, 2024 13:34

konstin force-pushed the konsti/publish4 branch from d9010a1 to d7b23e9 Compare September 19, 2024 13:35

lucab reviewed Sep 20, 2024

View reviewed changes

konstin had a problem deploying to uv-test-publish September 20, 2024 09:02 — with GitHub Actions Failure

konstin force-pushed the konsti/publish2 branch 2 times, most recently from 8f76504 to bad4fea Compare September 21, 2024 12:43

konstin force-pushed the konsti/publish4 branch from 3d0f533 to 30c52d1 Compare September 21, 2024 12:45

konstin force-pushed the konsti/publish2 branch 3 times, most recently from 7968483 to 2d5af07 Compare September 21, 2024 14:21

konstin force-pushed the konsti/publish4 branch from 30c52d1 to ec1ac14 Compare September 21, 2024 14:22

konstin had a problem deploying to uv-test-publish September 21, 2024 14:25 — with GitHub Actions Error

konstin force-pushed the konsti/publish4 branch from ec1ac14 to 23c6c30 Compare September 21, 2024 14:25

konstin marked this pull request as ready for review September 21, 2024 14:27

konstin requested review from zanieb and charliermarsh September 21, 2024 14:27

konstin added the enhancement New feature or improvement to existing functionality label Sep 21, 2024

konstin force-pushed the konsti/publish4 branch from 61c781e to 4e56a75 Compare September 21, 2024 14:32

konstin had a problem deploying to uv-test-publish September 21, 2024 14:36 — with GitHub Actions Failure

konstin had a problem deploying to uv-test-publish September 21, 2024 17:26 — with GitHub Actions Failure

konstin force-pushed the konsti/publish4 branch from 4cdefcf to 952694b Compare September 23, 2024 07:45

konstin had a problem deploying to uv-test-publish September 23, 2024 07:48 — with GitHub Actions Failure

konstin had a problem deploying to uv-test-publish September 23, 2024 07:55 — with GitHub Actions Failure

konstin mentioned this pull request Sep 23, 2024

Add uv publish: Basic upload with username/password or keyring #7475

Merged

konstin had a problem deploying to uv-test-publish September 23, 2024 10:26 — with GitHub Actions Failure

konstin force-pushed the konsti/publish4 branch from 97f53da to 26d2faf Compare September 23, 2024 10:57

konstin had a problem deploying to uv-test-publish September 23, 2024 11:03 — with GitHub Actions Failure

konstin force-pushed the konsti/publish4 branch from 26d2faf to 9bff75c Compare September 23, 2024 11:10

konstin temporarily deployed to uv-test-publish September 23, 2024 11:12 — with GitHub Actions Inactive

konstin force-pushed the konsti/publish4 branch from 9bff75c to 93c714d Compare September 23, 2024 11:30

konstin temporarily deployed to uv-test-publish September 23, 2024 11:32 — with GitHub Actions Inactive

charliermarsh reviewed Sep 23, 2024

View reviewed changes

crates/uv/src/commands/publish.rs Outdated Show resolved Hide resolved

charliermarsh reviewed Sep 23, 2024

View reviewed changes

crates/uv-settings/src/settings.rs Outdated Show resolved Hide resolved

charliermarsh approved these changes Sep 23, 2024

View reviewed changes

konstin force-pushed the konsti/publish2 branch from 33562d6 to 8ea5a02 Compare September 24, 2024 11:57

konstin force-pushed the konsti/publish4 branch 2 times, most recently from 81f3951 to 1f66b91 Compare September 24, 2024 14:58

konstin force-pushed the konsti/publish2 branch from e4a765f to 7eed9bd Compare September 24, 2024 15:24

Base automatically changed from konsti/publish2 to main September 24, 2024 15:33

konstin force-pushed the konsti/publish4 branch from a4de2fe to a1636c6 Compare September 24, 2024 15:47

konstin temporarily deployed to uv-test-publish September 24, 2024 15:50 — with GitHub Actions Inactive

konstin temporarily deployed to uv-test-publish September 24, 2024 15:58 — with GitHub Actions Inactive

konstin and others added 2 commits September 24, 2024 17:59

Add redundant check

9c91a1f

Update crates/uv-settings/src/Implement trusted publishing Co-authored-by: Charlie Marsh <[email protected]>

Newtype the token

ca550f2

konstin force-pushed the konsti/publish4 branch from 6798529 to ca550f2 Compare September 24, 2024 16:00

konstin enabled auto-merge (squash) September 24, 2024 16:00

konstin temporarily deployed to uv-test-publish September 24, 2024 16:03 — with GitHub Actions Inactive

konstin merged commit 205bf8c into main Sep 24, 2024
59 checks passed

konstin deleted the konsti/publish4 branch September 24, 2024 16:07

BrewTestBot mentioned this pull request Sep 24, 2024

uv 0.4.16 Homebrew/homebrew-core#191807

Merged

ElliottKasoar mentioned this pull request Jan 14, 2025

Fix publishing stfc/janus-core#381

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement trusted publishing #7548

Implement trusted publishing #7548

konstin commented Sep 19, 2024 •

edited

Loading

lucab Sep 20, 2024

lucab Sep 20, 2024

konstin Sep 20, 2024 •

edited

Loading

lucab Sep 20, 2024

lucab Sep 20, 2024 •

edited

Loading

charliermarsh Sep 23, 2024

konstin Sep 24, 2024

zanieb Sep 24, 2024

konstin Sep 24, 2024 •

edited

Loading

charliermarsh Sep 23, 2024

charliermarsh Sep 23, 2024

konstin Sep 24, 2024

charliermarsh Sep 24, 2024

charliermarsh Sep 23, 2024

konstin Sep 24, 2024

charliermarsh Sep 24, 2024

charliermarsh Sep 23, 2024

konstin Sep 24, 2024

charliermarsh Sep 23, 2024

konstin Sep 24, 2024

charliermarsh Sep 24, 2024

charliermarsh commented Sep 23, 2024

Implement trusted publishing #7548

Implement trusted publishing #7548

Conversation

konstin commented Sep 19, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

konstin Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucab Sep 20, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

konstin Sep 24, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

charliermarsh commented Sep 23, 2024

konstin commented Sep 19, 2024 •

edited

Loading

konstin Sep 20, 2024 •

edited

Loading

lucab Sep 20, 2024 •

edited

Loading

konstin Sep 24, 2024 •

edited

Loading