fix(ci): publish via uv to avoid OIDC token expiry#251
Conversation
📝 WalkthroughWalkthroughThe publish workflow's PyPI job now installs/sets up ChangesPublish workflow: uv publish
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/publish.yml:
- Line 347: The workflow currently disables attestations via the manifest key
attestations: false which weakens supply-chain guarantees; try alternatives
first: publish in batches (split the 21 artifacts and call the publish step
multiple times with fresh OIDC tokens), pre-compute/generate attestations in a
prior job if supported, reduce target wheel count, or add retry logic around the
publish step using the existing skip-existing: true to resume after OIDC expiry;
if none are feasible, add a clear comment immediately above the attestations:
false line documenting the operational constraint (OIDC token expiry), the
evaluated alternatives, and an explicit risk acceptance statement.
- Line 347: The workflow currently sets attestations: false which disables PEP
740 build provenance; update the publish workflow to document this decision and
add mitigation steps: replace the plain setting with an inline comment
explaining why attestations are disabled, add a TODO to re-enable them with a
mitigation plan (e.g., split the publish job into per-package jobs or smaller
batches, refresh/renew OIDC tokens, or adopt a signing step that can complete
before token expiry), and create a short checklist entry (in the repo README or
CONTRIBUTING) referencing the workflow symbol attestations so maintainers know
to revisit and re-enable attestations when the chosen mitigation (job-splitting
or token handling) is implemented.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 1f762157-e829-45af-b96b-20502ffca7ac
📒 Files selected for processing (1)
.github/workflows/publish.yml
gh-action-pypi-publish signs each artifact with Sigstore, exhausting the ~5min OIDC token before all 21 wheels upload (ExpiredIdentity). uv publish exchanges the token once and uploads without per-file signing.
193ee4f to
bd2dfff
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (1)
.github/workflows/publish.yml (1)
341-343:⚠️ Potential issue | 🟠 Major | ⚡ Quick winPin action version and uv binary to match CI workflow.
The setup-uv configuration differs from
.github/workflows/ci.ymlwhich usesastral-sh/setup-uv@v8.1.0withversion: "0.10.12". This workflow uses@v7without a version pin, creating two issues:
- Version mismatch: v7 vs v8.1.0 could cause different behavior between CI and publish.
- Missing uv version: Without
version, different uv releases may be used, risking publish inconsistencies.- Unpinned action: No commit hash pin (flagged by static analysis).
🔧 Suggested fix to align with CI
- name: Set up uv if: steps.pypi-check.outputs.all_uploaded != 'true' && !inputs.dry-run - uses: astral-sh/setup-uv@v7 + uses: astral-sh/setup-uv@v8.1.0 + with: + version: "0.10.12"For full supply-chain hardening, consider pinning to a commit hash as done with
actions/checkout.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish.yml around lines 341 - 343, The publish workflow uses astral-sh/setup-uv@v7 without a version field, causing a mismatch with the CI workflow (which uses astral-sh/setup-uv@v8.1.0 and version "0.10.12") and leaving the uv binary unpinned; update the publish job to use the same action reference and binary version as CI (use astral-sh/setup-uv@v8.1.0 and add version: "0.10.12") and for supply-chain hardening consider pinning the action to a specific commit hash (similar to actions/checkout) to avoid drift and ensure consistent publish behavior.Source: Linters/SAST tools
🧹 Nitpick comments (1)
.github/workflows/publish.yml (1)
345-347: 💤 Low valueCommand syntax is correct; attestation tradeoff remains.
The
uv publish --trusted-publishing always dist/*command correctly uses PyPI's OIDC-based trusted publishing (the requiredid-token: writepermission is declared at line 277). This sidesteps the OIDC token expiry issue sinceuvdoesn't perform sigstore attestation signing.Note: As discussed in prior review iterations, this approach removes PEP 740 attestations from published artifacts. The tradeoff is documented in the PR description, but consider adding a brief inline comment for future maintainers explaining why
uv publishwas chosen over the attestation-capable action.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/publish.yml around lines 345 - 347, Add a brief inline comment above the Publish to PyPI step explaining why we use the `uv publish --trusted-publishing always dist/*` command (it uses OIDC trusted publishing and avoids OIDC token expiry issues) and that this deliberately forgoes PEP 740/sigstore attestations; reference that the workflow has the `id-token: write` permission configured for OIDC and note the tradeoff for future maintainers so they understand why `uv publish` was chosen over an attestation-capable action.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In @.github/workflows/publish.yml:
- Around line 341-343: The publish workflow uses astral-sh/setup-uv@v7 without a
version field, causing a mismatch with the CI workflow (which uses
astral-sh/setup-uv@v8.1.0 and version "0.10.12") and leaving the uv binary
unpinned; update the publish job to use the same action reference and binary
version as CI (use astral-sh/setup-uv@v8.1.0 and add version: "0.10.12") and for
supply-chain hardening consider pinning the action to a specific commit hash
(similar to actions/checkout) to avoid drift and ensure consistent publish
behavior.
---
Nitpick comments:
In @.github/workflows/publish.yml:
- Around line 345-347: Add a brief inline comment above the Publish to PyPI step
explaining why we use the `uv publish --trusted-publishing always dist/*`
command (it uses OIDC trusted publishing and avoids OIDC token expiry issues)
and that this deliberately forgoes PEP 740/sigstore attestations; reference that
the workflow has the `id-token: write` permission configured for OIDC and note
the tradeoff for future maintainers so they understand why `uv publish` was
chosen over an attestation-capable action.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9e822ed6-8f95-42eb-8708-dd5464621496
📒 Files selected for processing (1)
.github/workflows/publish.yml
Summary
sigstore.oidc.ExpiredIdentitycrashed publish before any uploadTest plan
gh workflow run publish.yml -f version=0.16.1Summary by CodeRabbit