Conversation
There was a problem hiding this comment.
Pull request overview
Adds a cargo-dist configuration and an autogenerated GitHub Actions workflow to build and publish release artifacts via tag pushes.
Changes:
- Introduces
dist-workspace.tomlto configure cargo-dist (targets, installers, CI backend, custom runners). - Adds
.github/workflows/release.ymlto plan/build/host a cargo-dist release pipeline on GitHub Actions.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
dist-workspace.toml |
Defines cargo-dist workspace + distribution targets/installer settings used by CI. |
.github/workflows/release.yml |
Autogenerated release workflow to build artifacts and publish a GitHub Release. |
Comments suppressed due to low confidence (1)
.github/workflows/release.yml:297
- The
announcejob currently only checks out the repo and then exits; it doesn't download artifacts/manifests or perform any announcement/publish step. If announcements aren't needed, consider removing the job to reduce CI noise; otherwise add the missing announce steps (typically consuming the dist manifest and running the appropriatedistannounce/publish commands).
announce:
needs:
- plan
- host
# use "always() && ..." to allow us to wait for all publish jobs while
# still allowing individual publish jobs to skip themselves (for prereleases).
# "host" however must run to completion, no skipping allowed!
if: ${{ always() && needs.host.result == 'success' }}
runs-on: "ubuntu-22.04"
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
steps:
- uses: actions/checkout@v4
with:
persist-credentials: false
submodules: recursive
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| name: Release | ||
| permissions: | ||
| "contents": "write" | ||
|
|
There was a problem hiding this comment.
The workflow requests contents: write at the top-level while also running on pull_request. That gives PR jobs a write-capable GITHUB_TOKEN, which is unsafe when executing untrusted PR code (build scripts can call the GitHub API using the token). Set default permissions to read-only, and grant contents: write only on the tag-push publishing job(s) (e.g., job-level permissions on host / release creation).
| pull_request: | ||
| push: | ||
| tags: | ||
| - '**[0-9]+.[0-9]+.[0-9]+*' |
There was a problem hiding this comment.
on.push.tags uses a glob pattern, but this looks like a regex (+ quantifiers). In GitHub Actions tag filters, + is treated literally, so tags like 1.2.3/v1.2.3 will not match and the workflow won't run on releases. Use a glob that matches your release tags (or trigger on all tags and let dist validate).
| - '**[0-9]+.[0-9]+.[0-9]+*' | |
| - '*' |
| # we specify bash to get pipefail; it guards against the `curl` command | ||
| # failing. otherwise `sh` won't catch that `curl` returned non-0 | ||
| shell: bash | ||
| run: "curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/download/v0.30.3/cargo-dist-installer.sh | sh" |
There was a problem hiding this comment.
The curl | sh install pattern executes a remote script without integrity verification. Prefer using a pinned checksum/signature verification step, or install via a safer mechanism (e.g., downloading a specific release asset and verifying its SHA256) to reduce supply-chain risk.
| run: "curl --proto '=https' --tlsv1.2 -LsSf https://github.com/axodotdev/cargo-dist/releases/download/v0.30.3/cargo-dist-installer.sh | sh" | |
| run: | | |
| set -euo pipefail | |
| DIST_VERSION="v0.30.3" | |
| INSTALLER_URL="https://github.com/axodotdev/cargo-dist/releases/download/${DIST_VERSION}/cargo-dist-installer.sh" | |
| CHECKSUM_URL="${INSTALLER_URL}.sha256" | |
| INSTALLER_PATH="/tmp/cargo-dist-installer.sh" | |
| CHECKSUM_PATH="/tmp/cargo-dist-installer.sh.sha256" | |
| # Download installer script | |
| curl --proto '=https' --tlsv1.2 -LsSf "${INSTALLER_URL}" -o "${INSTALLER_PATH}" | |
| # Download corresponding SHA256 checksum | |
| curl --proto '=https' --tlsv1.2 -LsSf "${CHECKSUM_URL}" -o "${CHECKSUM_PATH}" | |
| # Verify checksum before executing | |
| (cd /tmp && sha256sum -c "$(basename "${CHECKSUM_PATH}")") | |
| # Execute the verified installer | |
| sh "${INSTALLER_PATH}" |
When the elapsed time is under 1 second (e.g., threshold set to 500ms and command took 800ms), show "800ms" instead of a confusing "0s". This addresses roborev finding #2 (medium severity). Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Handle poisoned mutex in recover_pending_termios with into_inner() to ensure terminal recovery even in edge cases (233 #1) - Add comment explaining double-close safety: longjmp skips File destructor so fd is still open and cannot be reused (233 #2) - Add comment explaining the intentional if guard in write_empty_password as release-build safety net (231 #2) Co-Authored-By: Claude Opus 4.6 <[email protected]>
- Fix fd leak when PENDING_TERMIOS_RESTORE mutex is poisoned after successful tcgetattr (finding #2) - Restore comment explaining intentional if guard in write_empty_password for release builds where debug_assert is stripped (finding #3) - Document that rpassword's internal fd leaks on longjmp as a known minor limitation (finding #1) - Note UTF-8 restriction from rpassword vs previous raw bytes (finding #4) Co-Authored-By: Claude Opus 4.6 <[email protected]>
No description provided.