033ecf5a4bce28abdcaeee7a18c4ededd2390d27
5 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Alex Waygood
|
033ecf5a4b |
Also have zizmor check for low-severity security issues (#14893)
## Summary This PR changes our zizmor configuration to also flag low-severity security issues in our GitHub Actions workflows. It's a followup to https://github.com/astral-sh/ruff/pull/14844. The issues being fixed here were all flagged by [zizmor's `template-injection` rule](https://woodruffw.github.io/zizmor/audits/#template-injection): > Detects potential sources of code injection via template expansion. > > GitHub Actions allows workflows to define template expansions, which occur within special `${{ ... }}` delimiters. These expansions happen before workflow and job execution, meaning the expansion of a given expression appears verbatim in whatever context it was performed in. > > Template expansions aren't syntax-aware, meaning that they can result in unintended shell injection vectors. This is especially true when they're used with attacker-controllable expression contexts, such as `github.event.issue.title` (which the attacker can fully control by supplying a new issue title). [...] > To fully remediate the vulnerability, you should not use `${{ env.VARNAME }}`, since that is still a template expansion. Instead, you should use `${VARNAME}` to ensure that the shell itself performs the variable expansion. ## Test Plan I tested that this passes all zizmore warnings by running `pre-commit run -a zizmor` locally. The other test is obviously to check that the workflows all still run correctly in CI 😄 |
||
|
Alex Waygood
|
58e7db89a1 |
Run zizmor in CI, and fix most warnings (#14844)
## Summary A [recent exploit](https://github.com/advisories/GHSA-7x29-qqmq-v6qc) brought attention to how easy it can be for attackers to use template expansion in GitHub Actions workflows to inject arbitrary code into a repository. That vulnerability [would have been caught by the zizmor linter](https://blog.yossarian.net/2024/12/06/zizmor-ultralytics-injection), which looks for potential security vulnerabilities in GitHub Actions workflows. This PR adds [zizmor](https://github.com/woodruffw/zizmor) as a pre-commit hook and fixes the high- and medium-severity warnings flagged by the tool. All the warnings fixed in this PR are related to this zizmor check: https://woodruffw.github.io/zizmor/audits/#artipacked. The summary of the check is that `actions/checkout` will by default persist git configuration for the duration of the workflow, which can be insecure. It's unnecessary unless you actually need to do things with `git` later on in the workflow. None of our workflows do except for `publish-docs.yml` and `sync-typeshed.yml`, so I set `persist-credentials: true` for those two but `persist-credentials: false` for all other uses of `actions/checkout`. Unfortunately there are several warnings in `release.yml`, including four high-severity warnings. However, this is a generated workflow file, so I have deliberately excluded this file from the check. These are the findings in `release.yml`: <details> <summary>release.yml findings</summary> ``` warning[artipacked]: credential persistence through GitHub Actions artifacts --> /Users/alexw/dev/ruff/.github/workflows/release.yml:62:9 | 62 | - uses: actions/checkout@v4 | _________- 63 | | with: 64 | | submodules: recursive | |_______________________________- does not set persist-credentials: false | = note: audit confidence → Low warning[artipacked]: credential persistence through GitHub Actions artifacts --> /Users/alexw/dev/ruff/.github/workflows/release.yml:124:9 | 124 | - uses: actions/checkout@v4 | _________- 125 | | with: 126 | | submodules: recursive | |_______________________________- does not set persist-credentials: false | = note: audit confidence → Low warning[artipacked]: credential persistence through GitHub Actions artifacts --> /Users/alexw/dev/ruff/.github/workflows/release.yml:174:9 | 174 | - uses: actions/checkout@v4 | _________- 175 | | with: 176 | | submodules: recursive | |_______________________________- does not set persist-credentials: false | = note: audit confidence → Low warning[artipacked]: credential persistence through GitHub Actions artifacts --> /Users/alexw/dev/ruff/.github/workflows/release.yml:249:9 | 249 | - uses: actions/checkout@v4 | _________- 250 | | with: 251 | | submodules: recursive 252 | | # Create a GitHub Release while uploading all files to it | |_______________________________________________________________- does not set persist-credentials: false | = note: audit confidence → Low error[excessive-permissions]: overly broad workflow or job-level permissions --> /Users/alexw/dev/ruff/.github/workflows/release.yml:17:1 | 17 | / permissions: 18 | | "contents": "write" ... | 39 | | # If there's a prerelease-style suffix to the version, then the release(s) 40 | | # will be marked as a prerelease. | |_________________________________^ contents: write is overly broad at the workflow level | = note: audit confidence → High error[template-injection]: code injection via template expansion --> /Users/alexw/dev/ruff/.github/workflows/release.yml:80:9 | 80 | - id: plan | _________^ 81 | | run: | | |_________^ 82 | || dist ${{ (inputs.tag && inputs.tag != 'dry-run' && format('host --steps=create --tag={0}', inputs.tag)) || 'plan' }} --out... 83 | || echo "dist ran successfully" 84 | || cat plan-dist-manifest.json 85 | || echo "manifest=$(jq -c "." plan-dist-manifest.json)" >> "$GITHUB_OUTPUT" | ||__________________________________________________________________________________^ this step | ||__________________________________________________________________________________^ inputs.tag may expand into attacker-controllable code | = note: audit confidence → Low error[template-injection]: code injection via template expansion --> /Users/alexw/dev/ruff/.github/workflows/release.yml:80:9 | 80 | - id: plan | _________^ 81 | | run: | | |_________^ 82 | || dist ${{ (inputs.tag && inputs.tag != 'dry-run' && format('host --steps=create --tag={0}', inputs.tag)) || 'plan' }} --out... 83 | || echo "dist ran successfully" 84 | || cat plan-dist-manifest.json 85 | || echo "manifest=$(jq -c "." plan-dist-manifest.json)" >> "$GITHUB_OUTPUT" | ||__________________________________________________________________________________^ this step | ||__________________________________________________________________________________^ inputs.tag may expand into attacker-controllable code | = note: audit confidence → Low error[template-injection]: code injection via template expansion --> /Users/alexw/dev/ruff/.github/workflows/release.yml:80:9 | 80 | - id: plan | _________^ 81 | | run: | | |_________^ 82 | || dist ${{ (inputs.tag && inputs.tag != 'dry-run' && format('host --steps=create --tag={0}', inputs.tag)) || 'plan' }} --out... 83 | || echo "dist ran successfully" 84 | || cat plan-dist-manifest.json 85 | || echo "manifest=$(jq -c "." plan-dist-manifest.json)" >> "$GITHUB_OUTPUT" | ||__________________________________________________________________________________^ this step | ||__________________________________________________________________________________^ inputs.tag may expand into attacker-controllable code | = note: audit confidence → Low ``` </details> ## Test Plan `uvx pre-commit run -a` |
||
|
samypr100
|
7dbd8f0f8e |
ci(docker): incorporate docker release enhancements from uv (#13274)
## Summary This PR updates `ruff` to match `uv` updated [docker releases approach](https://github.com/astral-sh/uv/blob/main/.github/workflows/build-docker.yml). It's a combined PR with changes from these PR's * https://github.com/astral-sh/uv/pull/6053 * https://github.com/astral-sh/uv/pull/6556 * https://github.com/astral-sh/uv/pull/6734 * https://github.com/astral-sh/uv/pull/7568 Summary of changes / features 1. This change would publish an additional tags that includes only `major.minor`. For a release with `x.y.z`, this would publish the tags: * ghcr.io/astral-sh/ruff:latest * ghcr.io/astral-sh/ruff:x.y.z * ghcr.io/astral-sh/ruff:x.y 2. Parallelizes multi-platform builds using multiple workers (hence the new docker-build / docker-publish jobs), which cuts docker releases time in half. 3. This PR introduces additional images with the ruff binaries from scratch for both amd64/arm64 and makes the mapping easy to configure by generating the Dockerfile on the fly. This approach focuses on minimizing CI time by taking advantage of dedicating a worker per mapping (20-30s~ per job). For example, on release `x.y.z`, this will publish the following image tags with format `ghcr.io/astral-sh/ruff:{tag}` with manifests for both amd64/arm64. This also include `x.y` tags for each respective additional tag. Note, this version does not include the python based images, unlike `uv`. * From **scratch**: `latest`, `x.y.z`, `x.y` (currently being published) * From **alpine:3.20**: `alpine`, `alpine3.20`, `x.y.z-alpine`, `x.y.z-alpine3.20` * From **debian:bookworm-slim**: `debian-slim`, `bookworm-slim`, `x.y.z-debian-slim`, `x.y.z-bookworm-slim` * From **buildpack-deps:bookworm**: `debian`, `bookworm`, `x.y.z-debian`, `x.y.z-bookworm` 4. This PR also fixes `org.opencontainers.image.version` for all tags (including the one from `scratch`) to contain the right release version instead of branch name `main` (current behavior). ``` > docker inspect ghcr.io/astral-sh/ruff:0.6.4 | jq -r '.[0].Config.Labels' { ... "org.opencontainers.image.version": "main" } ``` Closes https://github.com/astral-sh/ruff/issues/13481 ## Test Plan Approach mimics `uv` with almost no changes so risk is low but I still tested the full workflow. * I have a working CI release pipeline on my fork run https://github.com/samypr100/ruff/actions/runs/10966657733 * The resulting images were published to https://github.com/samypr100/ruff/pkgs/container/ruff |
||
|
renovate[bot]
|
85ede4a88c | Update docker/build-push-action action to v6 (#12127) | ||
|
Charlie Marsh
|
b0b68a5601 |
Migrate release workflow to cargo-dist (#9559)
## Summary This PR migrates our release workflow to [`cargo-dist`](https://github.com/axodotdev/cargo-dist). The primary motivation here is that we want to ship dedicated installers for Ruff that work across platforms, and `cargo-dist` gives us those installers out-of-the-box. The secondary motivation is that `cargo-dist` formalizes some of the patterns that we've built up over time in our own release process. At a high level: - The `release.yml` file is generated by `cargo-dist` with `cargo dist generate`. It doesn't contain any modifications vis-a-vis the generated file. (If it's edited out of band from generation, the release fails.) - Our customizations are inserted as custom steps within the `cargo-dist` workflow. Specifically, `build-binaries` builds the wheels and packages them into binaries (as on `main`), while `build-docker.yml` builds the Docker image. `publish-pypi.yml` publishes the wheels to PyPI. This is effectively our `release.yaml` (on `main`), broken down into individual workflows rather than steps within a single workflow. ### Changes from `main` The workflow is _nearly_ unchanged. We kick off a release manually via the GitHub Action by providing a tag. If the tag doesn't match the `Cargo.toml`, the release fails. If the tag matches an already-existing release, the release fails. The release proceeds by (in order): 0. Doing some upfront validation via `cargo-dist`. 1. Creating the wheels and archives. 2. Building and pushing the Docker image. 3. Publishing to PyPI (if it's not a "dry run"). 4. Creating the GitHub Release (if it's not a "dry run"). 5. Notifying `ruff-pre-commit` (if it's not a "dry run"). There are a few changes in the workflow as compared to `main`: - **We no longer validate the SHA** (just the tag). It's not an input to the job. The Axo team is considering whether / how to support this. - **Releases are now published directly** (rather than as draft). Again, the Axo team is considering whether / how to support this. The downside of drafts is that the URLs aren't stable, so the installers don't work _as long as the release is in draft_. This is fine for our workflow. It seems like the Axo team will add it. - Releases already contain the latest entry from the changelog (we don't need to copy it over). This "Just Works", which is nice, though we'll still want to edit them to add contributors. There are also a few **breaking changes** for consumers of the binaries: - **We no longer include the version tag in the file name**. This enables users to install via `/latest` URLs on GitHub, and is part of the cargo-dist paradigm. - **Archives now include an extra level of nesting,** which you can remove with `--strip-components=1` when untarring. Here's an example release that I created -- I omitted all the artifacts since I was just testing a workflow, so none of the installers or links work, but it gives you a sense for what the release looks like: https://github.com/charliermarsh/cargodisttest/releases/tag/0.1.13. ### Test Plan I ran a successful release to completion last night, and installed Ruff via the installer:   The piece I'm least confident about is the Docker push. We build the image, but the push fails in my test repo since I haven't wired up the credentials. |