8
\$\begingroup\$

I have a Github actions workflow that on pushing a tag does the following for a python package:

  1. Creates a Github release using the tag version.
  2. Publishes the package to PyPI.

The package build and publishing is done using uv.

Question: How can I improve this code? Would it be better to break this into two workflow files. One for Github release and one for PyPI publishing?

# .github/workflows/github_release_pypi_publish.yaml
name: Github Release

on:
  push:
    tags:
      - "[0-9]+.[0-9]+.[0-9]+" # Python versioning format

jobs:

  # Build the package
  build:
    name: Build dists
    runs-on: ubuntu-latest
    steps:
      - name: Checkout repo at tagged commit
      - uses: actions/checkout@v4

      - name: Install the latest version of uv
        uses: astral-sh/setup-uv@v6

      # Build wheels + sdist into ./dist
      - name: Build package with uv
        run: uv build

      # Persist artifacts for the next jobs
      - uses: actions/upload-artifact@v4
        with:
          name: dists
          path: dist/


  # Create a Github Release with the current tag version
  release:
    name: Create GitHub Release
    needs: build
    runs-on: ubuntu-latest
    permissions:
      contents: write
    
    steps:
      - name: Checkout repo to read from CHANGELOG.md
      - uses: actions/checkout@v4
      
      - name: Download build artifacts from the build job
      - uses: actions/download-artifact@v4
        with:
          name: dists
          path: dist/

      - name: Create Github Release for tag and attach built files
        uses: softprops/action-gh-release@v2
        with:
          tag_name: ${{ github.ref_name }}        # e.g. 1.2.3
          name: ${{ github.ref_name }}            # release title
          body_path: CHANGELOG.md                 # use the file as release notes
          files: |
            dist/*.whl
            dist/*.tar.gz


  # Publish package to PyPI
  publish:
    name: Publish to PyPI
    needs: release
    runs-on: ubuntu-latest
    permissions:
      id-token: write   # required for PyPI Trusted Publishing (OIDC)
    steps:
      - uses: actions/download-artifact@v4
        with:
          name: dists
          path: dist/

      - name: Install the latest version of uv
        uses: astral-sh/setup-uv@v6

      # Upload to PyPI via OIDC (no token needed once Trusted Publisher is configured)
      - name: uv publish
        run: uv publish
\$\endgroup\$

3 Answers 3

Reset to default
9
\$\begingroup\$

Correctness

Did you copy this code as-is? jobs.build.steps[0], jobs.release.steps[0] and jobs.release.steps[2] look suspicious, as if you didn't intend to start the next line with a hyphen. I'm not sure that a step can have only name...

  build:
    name: Build dists
    runs-on: ubuntu-latest
    steps:
      - name: Checkout repo at tagged commit
      - uses: actions/checkout@v4

      - name: Install the latest version of uv
        uses: astral-sh/setup-uv@v6

Security

There are several security-related issues in this workflow.

First, please make declaring top-level permissions a habit. If not provided, default permissions will be used for all jobs that do not override them, and you have such a job (build). Permissions should be predictable and follow a least privilege principle, while default GHA token has read access to almost everything. A reasonable choice for a deploy/test action would be

permissions:
  contents: read

at the top level - this removes all other permissions that might be available by default (such as issues).

The second similar issue is saving git credentials. If something goes wrong in your workflow with contents: write permission and you accidentally end up publishing your .git, an attacker can (over some limited time span) push something to your repository using those credentials. Not cool. Since you do not intend to git push, better just not save the credentials at all:

    - name: Checkout repo at tagged commit
      uses: actions/checkout@v4
      with:
        persist-credentials: false

The last problem is a tricky one: cache poisoning. You can read more about such attacks here (no affiliation, just a readable article). Avoid using caches in deploy workflows - I'm not absolutely sure that an attack is possible in your scenario, but better be cautious, you don't release so often that the cache benefits can become significant. Saving a minute from a release every week isn't worth a potential security breach. astral-sh/setup-uv enables cache for github-hosted runners by default AFAIC, but you can opt out explicitly.

Outdated actions

I see actions/checkout@v4 while v5 is already available - why? If this is a new code for review, better start with most recent dep versions... Same with actions/download-artifact.

Maybe: pin uv version

Just imagine: uv 0.8.16 is released with a critical bug that breaks uv publish completely. You happily dispatch the release workflow that was perfectly fine yesterday, and out of a sudden it dies hard. Oops. Ideally tool versions should be pinned and updated once in a while (but unfortunately neither dependabot nor renovate can handle such deps, so you need a manual solution or a scripted workflow).

Use automated tools

There are quite a few solutions that can check your GHA files for correctness and common vulnerabilities. I am personally familiar with check-jsonschema, actionlint (make sure to configure shellcheck with it) and zizmor. These three tools together can catch all invalid workflows, any issues shellcheck would report on your run steps (which is important when they become more complex than simple oneliners), and also some common security vulnerabilities. You can orchestrate them all as pre-commit hooks together with your python linter, achieving a very convenient lint setup. I have a working configuration for a tiny python project here (though there's no release workflow, it's not on PyPI, linking to pre-commit config).

Test before you publish

You have a separate build step - great! Now you can grab the result, uv pip install it in a separate job and run at least some basic tests to make sure that the resulting package is not corrupt. No need to run all your tests if they're too slow, but at least execute some simple "smoke-test" subset of them that validates most critical codebase parts.

Split?

Would it be better to break this into two workflow files

I'd say "no": you deploy the same thing, better build once and deploy everywhere. This leaves much less room for inconsistent artifacts available from different sources. This workflow is simple and tiny, and there's almost nothing to reuse, so keep all parts together (this will be easier to reason about, too).

\$\endgroup\$
2
  • \$\begingroup\$ Thank you for the detailed review! 1) Regarding correctness, yes the - with uses was a mistake, corrected. 2) I don't understand If not provided, default permissions will be used for all jobs that do not override them, and you have such a job (build). What are the default permissions? Where can I find details of this? 3) Regarding testing. If I also test after the build step, then should I split the script? Or the build->test->github release->pypi publish pipeline works well in a single script? \$\endgroup\$ Commented Sep 9 at 18:57
  • 1
    \$\begingroup\$ 2) This changelog entry from 2023 mentions that, but surprisingly I can't find the default list in the docs. Probably it's still "all scopes read-only". 3) Depends on the size of your test script - if it's long, I'd probably just put it in a separate .sh file to facilitate local testing, if it's just a few lines - add to the same workflow inline. \$\endgroup\$ Commented Sep 9 at 19:20
6
\$\begingroup\$

The workflow is well defined, steps clearly separated. The few comments you have are IMHO mostly unnecessary, as they state the same as the task they are commenting on. Maybe keep the one about the token when uploading to PyPI.

If you always publish to both GitHub and PyPI when you build, then I'd keep the workflow as-is. Otherwise you'd need a third flow orchestrating the show.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ Agree about the comments. The steps can have names, and most of them do. If it's important to know this information, put it in the name and delete the comment. If it's not, just delete the comment. Like in most of programming, good names can make most comments unnecessary. \$\endgroup\$ Commented Sep 9 at 16:08
5
\$\begingroup\$

Pin actions versions

Consider pinning your actions to a commit SHA:

Before:

- name: Install the latest version of uv
  uses: astral-sh/setup-uv@v6

After:

- name: Install the latest version of uv
  uses: astral-sh/setup-uv@557e51de59eb14aaaba2ed9621916900a91d50c6

Pinning to version tags is not enough because the attacker can update all the tags; this happened when tj-actions/changed-files was subject to a supply-chain attack in March 2025 (see this Issue). Similar attacks may happen to astral-sh or softprops in the future.

Pinning to a commit SHA is also recommended by GitHub.

\$\endgroup\$
1
  • 1
    \$\begingroup\$ This is a very nice suggestion, but you really must understand how it works. If you have an auto-upgrade tool like Renovate and merge its PRs blindly (or worse, auto-merge them if tests pass) - hash pinning adds zero protection, you'll happily bump to a "new" version hash pointing to a malicious commit. If you upgrade manually once in a while, you are still vulnerable, but to lesser extent - unless you've got enough time and expertise to validate a changeset between every two released versions, you can still upgrade to a hacked release. Hash pinning is as safe as is your upgrade process. \$\endgroup\$ Commented Sep 10 at 23:49

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.