Skip to content

Conversation

@nojaf
Copy link
Contributor

@nojaf nojaf commented Dec 5, 2025

Fixes #3203

@nojaf
Copy link
Contributor Author

nojaf commented Dec 5, 2025

Hi @Smaug123, I was inspired by how this was set up at GR.
Could you skim through this as a sanity check. (And approve if you can)

Copilot finished reviewing on behalf of nojaf December 5, 2025 14:01
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to set up trusted publishing for NuGet packages but doesn't fully implement the feature. The changes separate the release workflow from the main CI workflow and add explicit Build and Pack stages to the Release pipeline. However, the implementation still relies on API key authentication through the NuGet/login action rather than implementing true OIDC-based trusted publishing.

Key changes:

  • New dedicated release workflow (.github/workflows/release.yml) triggered on pushes to main
  • Release publishing removed from the main CI workflow
  • Build and Pack stages added to the Release pipeline in build.fsx

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.

File Description
.github/workflows/release.yml New workflow for NuGet package releases using NuGet/login action for API key retrieval
.github/workflows/main.yml Removed the publish step that ran on main branch pushes
build.fsx Added Build and Pack stages to the Release pipeline
CHANGELOG.md Added entry for version 7.0.4 documenting the trusted publishing feature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 23 to 31
- name: Obtain NuGet key
# this hash is v1.1.0
uses: NuGet/login@d22cc5f58ff5b88bf9bd452535b4335137e24544
id: login
with:
user: ${{ secrets.NUGET_USER }}
- name: "Publish"
env:
NUGET_KEY: ${{ steps.login.outputs.NUGET_API_KEY }}
Copy link

Copilot AI Dec 5, 2025

Choose a reason for hiding this comment

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

The implementation doesn't appear to use NuGet's trusted publishing (OIDC-based authentication). True trusted publishing would eliminate the need for API keys and the NuGet/login action. Instead, this still relies on the NUGET_API_KEY output from the login action, which defeats the purpose of trusted publishing.

For actual trusted publishing, you should:

  1. Configure trusted publishing in your NuGet.org account settings for this package
  2. Remove the NuGet/login action step
  3. Modify the dotnet nuget push command in pushPackage function to use the --skip-duplicate flag without requiring an API key when running in GitHub Actions with proper permissions

Reference: https://learn.microsoft.com/en-us/nuget/nuget-org/publish-with-github-actions

Suggested change
- name: Obtain NuGet key
# this hash is v1.1.0
uses: NuGet/login@d22cc5f58ff5b88bf9bd452535b4335137e24544
id: login
with:
user: ${{ secrets.NUGET_USER }}
- name: "Publish"
env:
NUGET_KEY: ${{ steps.login.outputs.NUGET_API_KEY }}
- name: "Publish"
env:

Copilot uses AI. Check for mistakes.
Copy link
Contributor

@Smaug123 Smaug123 left a comment

Choose a reason for hiding this comment

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

Looks correct to me assuming you're OK with the unconditional publish. Copilot is simply wrong about not requiring a NuGet API key at all.

DOTNET_CLI_TELEMETRY_OPTOUT: true

jobs:
build:
Copy link
Contributor

Choose a reason for hiding this comment

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

This release has become unconditional, if I understand correctly - personally I would prefer to ensure the tests have run first, but 🤷

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that is fair.

@nojaf nojaf merged commit 11df575 into fsprojects:main Dec 5, 2025
10 of 11 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.

Add Trusted Publishing

2 participants