Skip to content

Publish csharp from tag with Nuget OIDC - #1767

Merged
spacebear21 merged 2 commits into
payjoin:masterfrom
DanGould:nuget-oidc-publish
Aug 10, 2026
Merged

Publish csharp from tag with Nuget OIDC#1767
spacebear21 merged 2 commits into
payjoin:masterfrom
DanGould:nuget-oidc-publish

Conversation

@DanGould

Copy link
Copy Markdown
Member

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:

@DanGould
DanGould requested a review from chavic July 27, 2026 06:39
@coveralls

coveralls commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

Coverage Report for CI Build 30335256098

Coverage remained the same at 86.494%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: No coverable lines changed in this PR.
  • No coverage regressions found.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 16200
Covered Lines: 14012
Line Coverage: 86.49%
Coverage Strength: 340.8 hits per line

💛 - Coveralls

@chavic

chavic commented Jul 27, 2026

Copy link
Copy Markdown
Collaborator

cACK.

Two things:

  • Tag-workflow re-runs aren't idempotent: if publish succeeds but the release-attach job fails (or the run is re-triggered), dotnet nuget push will 409 on the already-published version and redden the whole run. --skip-duplicate makes re-runs converge instead.
  • The v-prefixed scheme diverges from the existing tags (payjoin-1.0.0-rc.5, payjoin-cli-1.0.0-rc.0), and Automate Package Publishing for All Release Artifacts #1766's payjoin-v* / payjoin-cli-v* won't match on tags that exist in the repo. Dropping the v with a payjoin-csharp-[0-9]* glob stays collision-free against payjoin-cli-* and bare payjoin-* tags, and lets the crates.io leg later trigger on the tags that already exist.

DanGould added 2 commits July 28, 2026 14:21
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.
@DanGould
DanGould force-pushed the nuget-oidc-publish branch from 917a522 to 163d7af Compare July 28, 2026 06:34
@DanGould
DanGould marked this pull request as ready for review July 29, 2026 06:29

@chavic chavic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 163d7af.

Five notes below. No note blocks the merge. Three come from this year's tj-actions and TanStack attacks.

# pack + every per-RID smoke job are green. Consumes the already-built,
# already-smoke-tested `payjoin-csharp-nuget-package` artifact — never repacks.
# ---------------------------------------------------------------------------
publish-nuget:

@chavic chavic Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread .github/workflows/csharp.yml
# 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

@chavic chavic Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)

@chavic chavic Aug 4, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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".

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These two comments say "See README". That text is in RELEASING.md.

@spacebear21
spacebear21 merged commit b4c2504 into payjoin:master Aug 10, 2026
37 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants