Skip to content

Sign released binaries with cert in Azure Key Vault - #1043

Merged
AArnott merged 1 commit into
mainfrom
aarnott-add-ev-package-signing
Aug 16, 2026
Merged

AArnott merged 1 commit into
mainfrom
aarnott-add-ev-package-signing

Conversation

@AArnott

@AArnott AArnott commented Aug 16, 2026

Copy link
Copy Markdown
Owner

No description provided.

Copilot AI lite review requested due to automatic review settings August 16, 2026 02:19
@AArnott
AArnott force-pushed the aarnott-add-ev-package-signing branch from 196f498 to 36a2ce8 Compare August 16, 2026 02:20

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Adds a Windows-based signing step to the release pipeline so that produced NuGet packages (and the DLLs inside them) are signed using a certificate stored in Azure Key Vault, enabling authenticated + timestamped signatures as part of publishing.

Changes:

  • Introduces tools/Sign-NuGetPackages.ps1 to extract .nupkg files, Authenticode-sign embedded DLLs via AzureSignTool, then re-pack and NuGet-sign packages via NuGetKeyVaultSignTool.
  • Updates the release workflow to run on Windows, authenticate to Azure via OIDC, and sign packages prior to dotnet nuget push.
  • Adds AzureSignTool and NuGetKeyVaultSignTool to the repo’s local dotnet tool manifest.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File Description
tools/Sign-NuGetPackages.ps1 New PowerShell script to sign DLLs and NuGet packages using Azure Key Vault-backed certificate.
.github/workflows/release.yml Runs release job on Windows; authenticates to Azure and invokes signing script before pushing packages.
.config/dotnet-tools.json Adds local dotnet tools required for signing (azuresigntool, nugetkeyvaultsigntool).

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tools/Sign-NuGetPackages.ps1 Outdated
Comment thread tools/Sign-NuGetPackages.ps1 Outdated
@codecov

codecov Bot commented Aug 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 76.36%. Comparing base (38624a1) to head (252cd4e).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1043      +/-   ##
==========================================
- Coverage   76.43%   76.36%   -0.07%     
==========================================
  Files         175      175              
  Lines       13382    13382              
  Branches     2709     2709              
==========================================
- Hits        10228    10219       -9     
- Misses       2276     2288      +12     
+ Partials      878      875       -3     
Flag Coverage Δ
Linux 75.20% <ø> (-0.09%) ⬇️
Windows 75.95% <ø> (-0.09%) ⬇️
macOS 75.24% <ø> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot AI review requested due to automatic review settings August 16, 2026 02:36
@AArnott
AArnott force-pushed the aarnott-add-ev-package-signing branch from 36a2ce8 to 252cd4e Compare August 16, 2026 02:36
@AArnott
AArnott enabled auto-merge August 16, 2026 02:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

Suppressed comments (4)

.github/workflows/release.yml:113

  • The push step uses Windows path separators, but the later Get-ChildItem in the same job still uses /deployables. Using a consistent path separator reduces confusion and avoids subtle path handling differences across tools.
      run: dotnet nuget push ${{ runner.temp }}\deployables\*.nupkg --source https://api.nuget.org/v3/index.json -k '${{ steps.nuget-login.outputs.NUGET_API_KEY }}'

tools/Sign-NuGetPackages.ps1:174

  • Resolve-Executable is asked to locate Azure CLI as az.cmd. On Windows this is often present, but some installations expose az (or az.exe) without an az.cmd shim, and the script already treats this as a Windows-only tool. Searching for az is more robust while still allowing an explicit path via -AzureCliPath.
$resolvedAzureCliPath = Resolve-Executable -Name 'az.cmd' -ExplicitPath $AzureCliPath
if (-not $resolvedAzureCliPath) {
    throw 'Azure CLI was not found. Install it or pass -AzureCliPath.'

.github/workflows/release.yml:102

  • This workflow now runs on Windows, but the signing step passes a mixed-separator path (.../deployables). While Windows often tolerates this, keeping \ consistently avoids edge cases in tooling and makes the script invocation clearer.

This issue also appears on line 113 of the same file.

        dotnet tool restore
        ./tools/Sign-NuGetPackages.ps1 `
          -Path '${{ runner.temp }}/deployables' `
          -KeyVaultUrl $env:AZURE_KEY_VAULT_URL `
          -CertificateName $env:AZURE_KEY_VAULT_CERTIFICATE

tools/Sign-NuGetPackages.ps1:182

  • The access token captured from az account get-access-token can come back with leading/trailing whitespace depending on how the command output is emitted. Trimming it makes the downstream --azure-key-vault-accesstoken argument more resilient.
$script:keyVaultAccessToken = (& $resolvedAzureCliPath account get-access-token `
    --resource 'https://vault.azure.net' `
    --query 'accessToken' `
    --output 'tsv')
if (($LASTEXITCODE -ne 0) -or -not $script:keyVaultAccessToken) {

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.

2 participants