ci: read NUGET_USER from a repository variable, not a secret - #413
Merged
Conversation
The nuget.org account name the trusted-publishing login authenticates as is a public profile name, not a credential: the API key it exchanges the OIDC token for is what must stay secret, and that key never leaves the job. Storing the username in Actions secrets bought no confidentiality and cost diagnosability — GitHub masks a secret in the logs, so a wrong or missing value showed up as an opaque login failure with the offending value redacted from the very output that would explain it. The value now lives in the repository variables, so the step reads it from the vars context. Documentation follows in both languages, in the release and dry-run pages, and the workflow comment records why it is a variable so it is not "hardened" back into a secret.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
release.ymlread the nuget.org account name fromsecrets.NUGET_USER, but that value now lives in the repository variables — the secret no longer exists.secrets.*does not fall back tovars.*: the expression resolved to an empty string, soNuGet/loginwould have run withuser: ""and failed the OIDC login on the next tag and on the next manual dry run. This points the step at thevarscontext and fixes the documentation, which described the value as a secret in four places.The username is a public nuget.org profile name, not a credential — the short-lived API key the OIDC exchange mints is what must stay secret, and it never leaves the job. Keeping the username in Actions secrets bought no confidentiality and cost diagnosability: GitHub masks a secret in the logs, so a wrong value surfaced as an opaque login failure with the offending value redacted from the very output that would explain it.
Type of change
Changes
.github/workflows/release.yml— theNuGet login (OIDC)step reads${{ vars.NUGET_USER }}instead of${{ secrets.NUGET_USER }}..github/workflows/release.yml— the step's comment names the repository variable and records why it is a variable rather than a secret, so it is not "hardened" back.doc/handwritten/for-maintainers/workflows/release.en.mdandrelease.fr.md— the Handle with care entry now requires the repository variable, with the same rationale.doc/handwritten/for-maintainers/ReleaseDryRun.en.mdandReleaseDryRun.fr.md— the dry-run side-effects note names the variable.No C# is touched, and no runtime, analyzer, or packaging behaviour changes.
Testing
dotnet build FirstClassErrors.slndotnet test FirstClassErrors.slnFirstClassErrors.Analyzers.UnitTests)The three boxes above were run on the runner, inside the dry run described below, at
-c Releaseon this exact commit — not locally.FirstClassErrors.Analyzers.UnitTestsis part ofFirstClassErrors.sln, so the solution-widedotnet testcovers it.Release dry run — green:
release.ymldispatched on this branch (lib, version0.0.0-dry.1,dry_runticked) — run 31245059241, 94s, jobPack & push NuGetsuccessful.NuGet login (OIDC)succeeded:Successfully exchanged OIDC token for NuGet API key.That is the end-to-end proof —vars.NUGET_USERresolves, and the nuget.org trusted-publishing policy accepts the exchange.user: SylvainAuratin clear where it used to printuser: ***. That is precisely the diagnosability this PR is about: a wrong value is now readable in the output that reports the failure.Push to NuGet,Publish GitHub Release,Refresh GenDoc's error-catalog baseline— skipped, as designed. Nothing was published.Require a major bump for GenDoc breaking changesskipped: it iscli-train only, which is why thelibtrain was chosen for this rehearsal.ReleaseDryRun.en.md: the run wrote a real provenance attestation for the throwaway0.0.0-dry.1to the attestation store and the public Sigstore log.Also run:
actionlint 1.7.7— the exact versionlint.ymlpins — over all workflows: clean, exit 0.Note for the merge:
pack-pushdeclares noenvironment:, soNUGET_USERmust stay a repository (or organization) variable — an environment-scoped variable would not be visible and would reproduce the empty-string failure this PR fixes. The green dry run confirms it is currently scoped correctly.Documentation
doc/updateddoc/handwritten/for-users/README.fr.md) updated if user-facing behavior changedThe maintainer documentation is updated in both languages (
release.{en,fr}.md,ReleaseDryRun.{en,fr}.md). The user-facingREADME.fr.mdis untouched: no user-facing behaviour changes.Architecture decisions
Proposed: ADR-____Checked against the ADR base. The lasting decision — trusted publishing with no long-lived
NUGET_API_KEYstored — is unchanged; this only corrects the classification of a value that was never a secret. The rationale lives in the workflow comment and the two documentation pages, which is where a future maintainer would look. Happy to draft one if you read it as a security-posture decision worth recording.Related issues
None.