Publish csharp from tag with Nuget OIDC - #1767
Conversation
Coverage Report for CI Build 30335256098Coverage remained the same at 86.494%Details
Uncovered ChangesNo uncovered changes found. Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
cACK. Two things:
|
Add a payjoin-csharp-v* tag trigger and two tag-gated jobs to the existing csharp.yml so the next C# release publishes from CI with no long-lived key: - publish-nuget: consumes the already-packed, already-smoke-tested payjoin-csharp-nuget-package artifact (no re-pack), verifies the tag matches the packed Payjoin.<version>.nupkg, attests build provenance, and pushes to nuget.org via NuGet/login trusted publishing (OIDC -> short-lived key). Runs in a nuget-release environment so a required reviewer approves before anything reaches nuget.org. - github-release: attaches the nupkg and a generated SHA256SUMS to the tag's GitHub release. The build/pack/smoke jobs are byte-identical to master; the publish path is skipped on every non-tag ref. softprops/action-gh-release is bumped v2 -> v3 to drop the deprecated Node 20 runtime.
Make the CI/OIDC tag flow the documented default and retitle the manual dotnet nuget push section "Manual fallback". Records the tag scheme, the nuget-release approval gate, the attestation-verify step, and the one-time Trusted Publishing policy binding.
917a522 to
163d7af
Compare
| # pack + every per-RID smoke job are green. Consumes the already-built, | ||
| # already-smoke-tested `payjoin-csharp-nuget-package` artifact — never repacks. | ||
| # --------------------------------------------------------------------------- | ||
| publish-nuget: |
There was a problem hiding this comment.
pack-nuget makes the bytes to be published, but It has no tag gate and no environment gate. The verify step checks the file name, not the content. So a bad change in pack-nuget reaches nuget.org with a valid attestation.
TanStack showed this. An attestation proves which pipeline built a package. It does not prove that the pipeline was correct.
Those jobs pin actions by tag, like the rest of the repo, so this is repo-wide and not a fault of this PR. I can open a separate issue for SHA pins on the publish path.
| # nuget.org policy to `nuget-release` AND add required reviewers, turning | ||
| # publish into a manual-approval gate. Create the environment first, or | ||
| # remove this line to publish without an approval gate. See README. | ||
| environment: nuget-release |
There was a problem hiding this comment.
An attacker can read an OIDC token from runner memory. TanStack used that method. This environment is the last manual control before a package goes to nuget.org. "Optional but recommended" may be too weak for that.
| name: payjoin-csharp-nuget-package | ||
| path: dist | ||
|
|
||
| - name: Generate SHA256SUMS (nupkg + each native lib) |
There was a problem hiding this comment.
The publish job checks that dist/ holds exactly one nupkg. This job does not. If dist/ holds two packages, unzip -q *.nupkg treats the second as a member pattern of the first, and the native hashes then cover one package only. should add the same check here, or pass the file name from the publish job.
| prerelease: ${{ contains(github.ref_name, '-preview') || contains(github.ref_name, '-rc') }} | ||
| # Dan's GPG detached signature over SHA256SUMS is added out-of-band | ||
| # (locally, then uploaded as SHA256SUMS.asc) — his private key must not | ||
| # live on a runner. See README "GPG signature". |
There was a problem hiding this comment.
These two comments say "See README". That text is in RELEASING.md.
The number of packages we have is mounting, so now we need a system for publishing packages and releasing. Since @chavic already did the groundwork for C sharp, this is the natural first step to #1766 imo. I think this current draft is a little bit too verbose in terms of its comments and documentation. looking for a concept ack first.
Co-authored by Claude code
Pull Request Checklist
Please confirm the following before requesting review:
AI
in the body of this PR.