From 9ff5d72784f32820eaf4c6e67db68b6a4f2e50cb Mon Sep 17 00:00:00 2001 From: Trent Blackburn Date: Thu, 27 Aug 2026 16:11:09 -0400 Subject: [PATCH] fix: Make Sign.SkipCertificateValidation actually skip validation The setting was documented for the Store and Thumbprint sources and worked for neither. Get-PSBuildCertificate consulted -SkipValidation only for the EnvVar and PfxFile sources, which load exactly one certificate; Store and Thumbprint checked HasPrivateKey and NotAfter inside the Where-Object that selects the certificate, where the switch could not reach them. An expired store certificate was dropped before validation was ever considered and the build failed with NoCertificateFound, which points at a missing certificate rather than at the expiry that caused it. Naively hoisting those checks out of the filter would have changed selection: Select-Object -First 1 could then return an expired certificate even when a valid one exists further down the store. So the relaxation is a fallback instead. A valid certificate is preferred always, and a merely-present one is accepted only when nothing valid was found and the consumer explicitly opted out. The Thumbprint fallback keeps the thumbprint match and drops only the expiry check, because returning a certificate other than the one the consumer named would sign with a different identity than the build asked for. HasPrivateKey stays required in both: a certificate without a private key cannot sign anything, so skipping that check would buy nothing and only defer the failure to Set-AuthenticodeSignature with a worse error. Falling back now warns, since the consumer is about to sign with an expired certificate. Separately, IB.tasks.ps1 never passed the setting to Get-PSBuildCertificate in either of its signing tasks, so it was dead on all four sources for Invoke-Build consumers regardless. That is the same divergence between the two task files as #178, and no test could see it, so IBTasks.tests.ps1 now compares the $PSBPreference.Sign settings each task file reads. The comparison is scoped to Sign because the two files legitimately differ elsewhere. SkipCertificateValidation was the only signing setting missing from either. Closes #193 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01U1Jhu7fgTRJq7LK5MuKteE --- CHANGELOG.md | 19 +++ PowerShellBuild/IB.tasks.ps1 | 2 + .../Public/Get-PSBuildCertificate.ps1 | 57 ++++++- PowerShellBuild/build.properties.ps1 | 21 ++- PowerShellBuild/en-US/Messages.psd1 | 1 + tests/Get-PSBuildCertificate.tests.ps1 | 141 ++++++++++++++++++ tests/IBTasks.tests.ps1 | 46 ++++++ 7 files changed, 277 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0baa132..5ef8149 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -77,6 +77,25 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ### Fixed +- [**#193**](https://github.com/psake/PowerShellBuild/issues/193) + `$PSBPreference.Sign.SkipCertificateValidation` now does something. It was + read only by `psakeFile.ps1`, and even there `Get-PSBuildCertificate` consulted + it only for the `EnvVar` and `PfxFile` sources — the `Store` and `Thumbprint` + sources checked expiry and private key presence inside the filter that selects + the certificate, where the switch could not reach them. So the documented + escape hatch did nothing on two of the four sources for psake consumers, and + nothing at all for Invoke-Build consumers, because `IB.tasks.ps1` never passed + the setting to `Get-PSBuildCertificate` in either of its signing tasks. A + build with an expired store certificate failed with `NoCertificateFound`, + which points at a missing certificate rather than at the expiry that actually + caused it. `IB.tasks.ps1` now passes the setting, and for `Store` and + `Thumbprint` the relaxation is a fallback rather than a blanket bypass: an + unexpired certificate is still preferred whenever one exists, an expired one + is selected only when no unexpired one was found, and a warning is emitted + when that happens. `Thumbprint` still matches the requested thumbprint, and a + private key is still required in every case, because a certificate without one + cannot sign. + - [**#124**](https://github.com/psake/PowerShellBuild/issues/124) The docs tree can hold documentation that is not generated help. A `README.md` at its root, a `CONTRIBUTING.md` beside the generated markdown, an `images/` or diff --git a/PowerShellBuild/IB.tasks.ps1 b/PowerShellBuild/IB.tasks.ps1 index 1142bfc..e299ffa 100644 --- a/PowerShellBuild/IB.tasks.ps1 +++ b/PowerShellBuild/IB.tasks.ps1 @@ -219,6 +219,7 @@ Task SignModule -If { CertStoreLocation = $PSBPreference.Sign.CertStoreLocation CertificateEnvVar = $PSBPreference.Sign.CertificateEnvVar CertificatePasswordEnvVar = $PSBPreference.Sign.CertificatePasswordEnvVar + SkipValidation = $PSBPreference.Sign.SkipCertificateValidation } if ($PSBPreference.Sign.Thumbprint) { $certParams.Thumbprint = $PSBPreference.Sign.Thumbprint @@ -294,6 +295,7 @@ Task SignCatalog -If { CertStoreLocation = $PSBPreference.Sign.CertStoreLocation CertificateEnvVar = $PSBPreference.Sign.CertificateEnvVar CertificatePasswordEnvVar = $PSBPreference.Sign.CertificatePasswordEnvVar + SkipValidation = $PSBPreference.Sign.SkipCertificateValidation } if ($PSBPreference.Sign.Thumbprint) { $certParams.Thumbprint = $PSBPreference.Sign.Thumbprint diff --git a/PowerShellBuild/Public/Get-PSBuildCertificate.ps1 b/PowerShellBuild/Public/Get-PSBuildCertificate.ps1 index c8bdbec..2b97c49 100644 --- a/PowerShellBuild/Public/Get-PSBuildCertificate.ps1 +++ b/PowerShellBuild/Public/Get-PSBuildCertificate.ps1 @@ -51,9 +51,14 @@ function Get-PSBuildCertificate { .PARAMETER PfxFilePassword Password for the PFX file as a SecureString. Used by PfxFile source. .PARAMETER SkipValidation - Skip validation checks (private key presence, expiration, Code Signing EKU) for certificates - loaded from EnvVar or PfxFile sources. Use with caution; invalid certificates will fail during - actual signing operations with less descriptive errors. + Relax the certificate validity checks. For the EnvVar and PfxFile sources, which load + exactly one certificate, this skips the private key, expiration, and Code Signing EKU + checks outright. For the Store and Thumbprint sources, which select one certificate out + of many, an unexpired certificate is still preferred whenever one exists; an expired + certificate is returned only when no unexpired one was found, and a warning is emitted + when that happens. A private key is required in every case, because a certificate + without one cannot sign. Use with caution; invalid certificates will fail during actual + signing operations with less descriptive errors. .OUTPUTS System.Security.Cryptography.X509Certificates.X509Certificate2 Returns the resolved certificate, or $null if none was found (Store/Thumbprint sources). @@ -129,9 +134,30 @@ function Get-PSBuildCertificate { if ($null -ne $IsWindows -and -not $IsWindows) { throw $LocalizedData.CertificateSourceStoreNotSupported } - $cert = Get-ChildItem -Path $CertStoreLocation -CodeSigningCert | + $candidateCertificate = Get-ChildItem -Path $CertStoreLocation -CodeSigningCert + + # The store holds many certificates, so validity is part of how the right one is + # selected rather than a gate applied to a single loaded certificate. Prefer a valid + # certificate first, always. + $cert = $candidateCertificate | Where-Object { $_.HasPrivateKey -and $_.NotAfter -gt (Get-Date) } | Select-Object -First 1 + + # Only when the consumer explicitly opted out of validation does an expired + # certificate become acceptable, and only as a fallback, so a valid certificate is + # never passed over in favour of an expired one further down the store. + # HasPrivateKey stays required: a certificate without one cannot sign anything, so + # relaxing that check would buy nothing and only defer the failure to + # Set-AuthenticodeSignature with a less descriptive error. + if (-not $cert -and $SkipValidation) { + $cert = $candidateCertificate | + Where-Object { $_.HasPrivateKey } | + Select-Object -First 1 + if ($cert) { + Write-Warning ($LocalizedData.CertificateValidationRelaxed -f $cert.NotAfter, $cert.Subject) + } + } + if ($cert) { Write-Verbose ($LocalizedData.CertificateResolvedFromStore -f $CertStoreLocation, $cert.Subject) } @@ -144,13 +170,34 @@ function Get-PSBuildCertificate { # Normalize thumbprint input by removing whitespace for robust matching $normalizedThumbprint = ($Thumbprint -replace '\s', '') - $cert = Get-ChildItem -Path $CertStoreLocation -CodeSigningCert | + $candidateCertificate = Get-ChildItem -Path $CertStoreLocation -CodeSigningCert + + # As with the Store source, validity is part of the selection. Prefer a valid + # certificate first, always. + $cert = $candidateCertificate | Where-Object { ($_.Thumbprint -replace '\s', '') -ieq $normalizedThumbprint -and $_.HasPrivateKey -and $_.NotAfter -gt (Get-Date) } | Select-Object -First 1 + + # The fallback relaxes only the expiry check. The thumbprint match is kept, because + # returning a certificate other than the one the consumer named would sign with a + # different identity than the build asked for, and HasPrivateKey is kept for the + # same reason it is kept in the Store source. + if (-not $cert -and $SkipValidation) { + $cert = $candidateCertificate | + Where-Object { + ($_.Thumbprint -replace '\s', '') -ieq $normalizedThumbprint -and + $_.HasPrivateKey + } | + Select-Object -First 1 + if ($cert) { + Write-Warning ($LocalizedData.CertificateValidationRelaxed -f $cert.NotAfter, $cert.Subject) + } + } + if ($cert) { Write-Verbose ($LocalizedData.CertificateResolvedFromThumbprint -f $Thumbprint, $cert.Subject) } diff --git a/PowerShellBuild/build.properties.ps1 b/PowerShellBuild/build.properties.ps1 index 2e8d24c..23a5ca9 100644 --- a/PowerShellBuild/build.properties.ps1 +++ b/PowerShellBuild/build.properties.ps1 @@ -186,11 +186,22 @@ $moduleVersion = (Import-PowerShellDataFile -Path $env:BHPSModuleManifest).Modul # Useful for Azure Key Vault, HSM, or other custom certificate providers. Certificate = $null - # When true and using the Store or Thumbprint sources, skip the - # certificate validity check that ensures the certificate is not expired - # and has a private key. This is not recommended for production use but - # can be useful in CI environments where certificates are frequently - # renewed and updated. + # When true, relax the certificate validity checks. This is not + # recommended for production use but can be useful in CI environments + # where certificates are frequently renewed and rotated. + # + # The EnvVar and PfxFile sources load exactly one certificate, so the + # expiration and Code Signing EKU checks are skipped outright for them. + # + # The Store and Thumbprint sources select one certificate out of many, + # so the relaxation is a fallback rather than a blanket bypass: an + # unexpired certificate is preferred whenever one exists, and an expired + # one is returned only when no unexpired certificate was found. A + # warning is emitted when that happens, and the Thumbprint source still + # matches the requested thumbprint. + # + # A private key is required in every case, because a certificate without + # one cannot sign. SkipCertificateValidation = $false # RFC 3161 timestamp server URI embedded in Authenticode signatures. diff --git a/PowerShellBuild/en-US/Messages.psd1 b/PowerShellBuild/en-US/Messages.psd1 index 0feb84b..0daf1e8 100644 --- a/PowerShellBuild/en-US/Messages.psd1 +++ b/PowerShellBuild/en-US/Messages.psd1 @@ -40,4 +40,5 @@ CertificateMissingPrivateKey=The resolved certificate does not have an accessibl CertificateExpired=The resolved certificate has expired (NotAfter: {0}). Code signing requires a valid, unexpired certificate. Subject=[{1}] CertificateMissingCodeSigningEku=The resolved certificate does not have the Code Signing Enhanced Key Usage (EKU: 1.3.6.1.5.5.7.3.3). Subject=[{0}] CertificateSourceStoreNotSupported=CertificateSource 'Store' is only supported on Windows platforms. +CertificateValidationRelaxed=No unexpired code signing certificate was found, and validation was skipped, so an expired certificate was selected (NotAfter: {0}). Subject=[{1}] '@ diff --git a/tests/Get-PSBuildCertificate.tests.ps1 b/tests/Get-PSBuildCertificate.tests.ps1 index bf9f9e6..317f657 100644 --- a/tests/Get-PSBuildCertificate.tests.ps1 +++ b/tests/Get-PSBuildCertificate.tests.ps1 @@ -194,5 +194,146 @@ Describe 'Code Signing Functions' { Should -BeFalse } } + + # The Store and Thumbprint sources select one certificate out of many, so validity is part + # of the selection rather than a gate applied to a single loaded certificate. SkipValidation + # therefore relaxes the selection only as a fallback: a valid certificate is preferred + # whenever one exists, and an expired one is accepted only when nothing valid was found. + # See psake/PowerShellBuild#193. + # + # These tests mock Get-ChildItem inside the module session state, which is the only way the + # mock reaches the call made by Get-PSBuildCertificate. They are Windows-only because the + # -CodeSigningCert dynamic parameter comes from the certificate provider, which exists only + # on Windows, and because the Store source throws on other platforms by design. + Context 'SkipValidation for store-backed sources' { + + BeforeAll { + $script:validCertificate = [PSCustomObject]@{ + Subject = 'CN=Valid Test Certificate' + Thumbprint = 'AAAA111122223333444455556666777788889999' + HasPrivateKey = $true + NotAfter = (Get-Date).AddDays(30) + } + $script:expiredCertificate = [PSCustomObject]@{ + Subject = 'CN=Expired Test Certificate' + Thumbprint = 'BBBB111122223333444455556666777788889999' + HasPrivateKey = $true + NotAfter = (Get-Date).AddDays(-1) + } + $script:noPrivateKeyCertificate = [PSCustomObject]@{ + Subject = 'CN=No Private Key Test Certificate' + Thumbprint = 'CCCC111122223333444455556666777788889999' + HasPrivateKey = $false + NotAfter = (Get-Date).AddDays(30) + } + } + + Context 'Store source' { + It 'Prefers a valid certificate over an expired one even when SkipValidation is set' -Skip:(-not $IsWindows) { + # The expired certificate is returned first so that a naive implementation, one that + # hoists the validity checks out of the selection filter, would pick it. + Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem -MockWith { + $script:expiredCertificate + $script:validCertificate + } + + $certificate = Get-PSBuildCertificate -CertificateSource Store -SkipValidation + + $certificate.Subject | Should -Be 'CN=Valid Test Certificate' + } + + It 'Returns an expired certificate when SkipValidation is set and nothing valid is available' -Skip:(-not $IsWindows) { + Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem -MockWith { + $script:expiredCertificate + } + + $certificate = Get-PSBuildCertificate -CertificateSource Store -SkipValidation -WarningAction SilentlyContinue + + $certificate.Subject | Should -Be 'CN=Expired Test Certificate' + } + + It 'Warns when SkipValidation causes an expired certificate to be selected' -Skip:(-not $IsWindows) { + Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem -MockWith { + $script:expiredCertificate + } + + Get-PSBuildCertificate -CertificateSource Store -SkipValidation ` + -WarningVariable warningRecord -WarningAction SilentlyContinue | Out-Null + + $warningRecord -join ' ' | Should -Match 'expired' + } + + It 'Does not return an expired certificate when SkipValidation is not set' -Skip:(-not $IsWindows) { + Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem -MockWith { + $script:expiredCertificate + } + + $certificate = Get-PSBuildCertificate -CertificateSource Store + + $certificate | Should -BeNullOrEmpty + } + + It 'Does not return a certificate without a private key even when SkipValidation is set' -Skip:(-not $IsWindows) { + # A certificate with no private key cannot sign anything, so relaxing that check + # would only defer the failure to Set-AuthenticodeSignature with a worse message. + Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem -MockWith { + $script:noPrivateKeyCertificate + } + + $certificate = Get-PSBuildCertificate -CertificateSource Store -SkipValidation -WarningAction SilentlyContinue + + $certificate | Should -BeNullOrEmpty + } + } + + Context 'Thumbprint source' { + It 'Returns an expired certificate when SkipValidation is set and nothing valid is available' -Skip:(-not $IsWindows) { + Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem -MockWith { + $script:expiredCertificate + } + + $certificate = Get-PSBuildCertificate -CertificateSource Thumbprint ` + -Thumbprint $script:expiredCertificate.Thumbprint -SkipValidation -WarningAction SilentlyContinue + + $certificate.Subject | Should -Be 'CN=Expired Test Certificate' + } + + It 'Still honours the requested thumbprint when SkipValidation relaxes the selection' -Skip:(-not $IsWindows) { + # A valid certificate is present, but it is not the one the consumer named. Returning + # it would sign with a different identity than the build asked for. + Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem -MockWith { + $script:validCertificate + $script:expiredCertificate + } + + $certificate = Get-PSBuildCertificate -CertificateSource Thumbprint ` + -Thumbprint $script:expiredCertificate.Thumbprint -SkipValidation -WarningAction SilentlyContinue + + $certificate.Thumbprint | Should -Be $script:expiredCertificate.Thumbprint + } + + It 'Does not return an expired certificate when SkipValidation is not set' -Skip:(-not $IsWindows) { + Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem -MockWith { + $script:expiredCertificate + } + + $certificate = Get-PSBuildCertificate -CertificateSource Thumbprint ` + -Thumbprint $script:expiredCertificate.Thumbprint + + $certificate | Should -BeNullOrEmpty + } + + It 'Does not return a certificate without a private key even when SkipValidation is set' -Skip:(-not $IsWindows) { + Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem -MockWith { + $script:noPrivateKeyCertificate + } + + $certificate = Get-PSBuildCertificate -CertificateSource Thumbprint ` + -Thumbprint $script:noPrivateKeyCertificate.Thumbprint -SkipValidation -WarningAction SilentlyContinue + + $certificate | Should -BeNullOrEmpty + } + } + } } } diff --git a/tests/IBTasks.tests.ps1 b/tests/IBTasks.tests.ps1 index 2ceb104..3cf8e95 100644 --- a/tests/IBTasks.tests.ps1 +++ b/tests/IBTasks.tests.ps1 @@ -99,3 +99,49 @@ Describe 'Settings referenced by the task files' { $unresolvable -join ', ' | Should -BeNullOrEmpty } } + +Describe 'Signing settings referenced by the task files' { + + # Both task files build a certificate parameter hashtable and splat it onto + # Get-PSBuildCertificate, and until psake/PowerShellBuild#193 nothing compared the two + # bodies. IB.tasks.ps1 never passed SkipValidation, so + # $PSBPreference.Sign.SkipCertificateValidation was dead for every Invoke-Build consumer + # while it worked for psake consumers. The task-name comparison above cannot see that, + # because both files define the same tasks. + # + # The comparison is scoped to the $PSBPreference.Sign settings rather than every + # $PSBPreference setting because the two files legitimately differ elsewhere: only + # IB.tasks.ps1 reads Build.Dependencies, and only psakeFile.ps1 uses the + # $PSB{TaskName}Dependency variables. + + BeforeAll { + $script:moduleSourcePath = [IO.Path]::Combine( + (Split-Path -Path $PSScriptRoot -Parent), 'PowerShellBuild' + ) + + function script:Get-SignSettingName { + param([string]$FileName) + + $taskFileContent = Get-Content -Path ( + [IO.Path]::Combine($script:moduleSourcePath, $FileName) + ) -Raw + + [regex]::Matches( + $taskFileContent, '\$PSBPreference\.Sign((?:\.[A-Za-z_][A-Za-z0-9_]*)+)' + ).ForEach({ $_.Groups[1].Value.TrimStart('.') }) | Sort-Object -Unique + } + } + + It 'reads the same $PSBPreference.Sign settings in both task files' { + $psakeSetting = Get-SignSettingName -FileName 'psakeFile.ps1' + $invokeBuildSetting = Get-SignSettingName -FileName 'IB.tasks.ps1' + + $psakeSetting | Should -Not -BeNullOrEmpty -Because 'the regex must still match something' + + $missingFromInvokeBuild = $psakeSetting.Where({ $_ -notin $invokeBuildSetting }) + $missingFromPsake = $invokeBuildSetting.Where({ $_ -notin $psakeSetting }) + + $missingFromInvokeBuild -join ', ' | Should -BeNullOrEmpty -Because 'IB.tasks.ps1 must honour every signing setting psakeFile.ps1 honours' + $missingFromPsake -join ', ' | Should -BeNullOrEmpty -Because 'psakeFile.ps1 must honour every signing setting IB.tasks.ps1 honours' + } +}