fix: Make Sign.SkipCertificateValidation actually skip validation

[tablackburn]

Summary

Closes #193. Two defects, one root cause each.

• Get-PSBuildCertificate never let -SkipValidation reach the Store and
  Thumbprint sources. Those two branches checked HasPrivateKey and
  NotAfter inside the Where-Object that selects the certificate, where the
  switch cannot reach them. The relaxation is now a fallback: 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.
• IB.tasks.ps1 never passed the setting at all. Both of its $certParams
  blocks omitted SkipValidation, so $PSBPreference.Sign.SkipCertificateValidation
  was dead on all four sources for Invoke-Build consumers. Both blocks now pass it.
• build.properties.ps1 describes what the code now actually does, including
  the fallback semantics.
• tests/IBTasks.tests.ps1 gains a comparison of the $PSBPreference.Sign
  settings each task file reads, so this class of divergence cannot recur.

Why a fallback rather than a hoist

Store and Thumbprint select one certificate from many, so filtering is
how the right one gets picked. EnvVar and PfxFile load exactly one, so
validation there is a gate on the only candidate. Hoisting the 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. The
fallback preserves the selection semantics and still serves the CI case the
setting exists for.

The Thumbprint fallback keeps the thumbprint match and drops only the expiry
condition. Selecting a different certificate than the one the consumer named
would sign with a different identity than the build asked for.

HasPrivateKey stays required — confirmed

The snippet in the issue keeps requiring it, and that is right. A certificate
with no private key cannot sign anything. Relaxing that check buys nothing: it
does not turn a failing build into a passing one, it only moves the failure from
Get-PSBuildCertificate, which can say this certificate has no private key,
to Set-AuthenticodeSignature, which cannot. SkipCertificateValidation exists
to unblock a build whose certificate has just expired or is mid-rotation — a
real, recoverable situation. A certificate with no private key is not that.

Two tests pin this down, one per source: Does not return a certificate without
a private key even when SkipValidation is set.

The fallback warns

Yes. The consumer is about to sign with an expired certificate, and the whole
reason the current behavior is a bug is that the failure mode was silent —
Store and Thumbprint dropped the certificate and fell through to the generic
NoCertificateFound, saying nothing about the expiry. Trading one silence for
another would be a poor fix.

The message follows the module's convention: a new CertificateValidationRelaxed
key in en-US/Messages.psd1, looked up through $LocalizedData. It reports the
NotAfter and the subject, so the build log names the certificate and the date.
Write-Warning, not Write-Verbose — this should be visible in a default CI log.

Per the task note, #187 changes how $LocalizedData loads; nothing here touches
that loading, only the data file, so the new key should not conflict.

Task-file divergence sweep

Every $PSBPreference.Sign.* read in both files, compared:

Setting	psakeFile.ps1	IB.tasks.ps1 (before)
Enabled, CertificateSource, CertStoreLocation, CertificateEnvVar, CertificatePasswordEnvVar, Thumbprint, PfxFilePath, PfxFilePassword, Certificate, TimestampServer, HashAlgorithm, FilesToSign, Catalog.Enabled, Catalog.Version, Catalog.FileName	✅	✅
SkipCertificateValidation	✅	❌

SkipCertificateValidation was the only one missing, in both the SignModule
and SignCatalog bodies. Nothing was missing in the other direction.

One non-$PSBPreference difference is legitimate and was left alone:
psakeFile.ps1 adds Verbose = $VerbosePreference -eq 'Continue' to every
parameter hashtable and IB.tasks.ps1 does not. That is a task-runner
difference, not a setting, and it is consistent across all four of the psake
signing hashtables rather than a one-off omission.

How the certificate store is mocked

The existing 17 tests do Mock Get-ChildItem { } without -ModuleName, so the
mock never reaches the call inside the module — those tests assert $null and
would pass against a real, empty store, which is why they never caught this. The
new tests use Mock -ModuleName PowerShellBuild -CommandName Get-ChildItem,
which does reach it, and return [PSCustomObject] certificates carrying just
Subject, Thumbprint, HasPrivateKey, and NotAfter. That was verified to
work, -CodeSigningCert dynamic parameter included, before any test was written.

They are -Skip:(-not $IsWindows), matching every existing Store/Thumbprint
test: -CodeSigningCert comes from the certificate provider, which exists only
on Windows, and the Store source throws on other platforms by design.

Red before green

Against the unfixed code, with the new tests in place
(Invoke-Pester ./tests/Get-PSBuildCertificate.tests.ps1,./tests/IBTasks.tests.ps1):

 [-] Store source.Returns an expired certificate when SkipValidation is set and nothing valid is available
  Expected 'CN=Expired Test Certificate', but got $null.
 [-] Store source.Warns when SkipValidation causes an expired certificate to be selected
  Expected regular expression 'expired' to match <empty>, but it did not match.
 [-] Thumbprint source.Returns an expired certificate when SkipValidation is set and nothing valid is available
  Expected 'CN=Expired Test Certificate', but got $null.
 [-] Thumbprint source.Still honours the requested thumbprint when SkipValidation relaxes the selection
  Expected 'BBBB111122223333444455556666777788889999', but got $null.
 [-] reads the same $PSBPreference.Sign settings in both task files
  Expected $null or empty, because IB.tasks.ps1 must honour every signing setting
  psakeFile.ps1 honours, but got 'SkipCertificateValidation'.

Every behavior fixed by this PR fails first. All five pass after.

The sixth new behavior — Prefers a valid certificate over an expired one even
when SkipValidation is set — is the regression the fallback design exists to
prevent, so it is green against the unfixed code by construction. Green-before
proves nothing there, so it was validated against the naive hoist instead:
replacing the Store filter with Get-ChildItem ... | Select-Object -First 1
and moving validation to the shared block, which is the obvious alternative fix.

 [-] Prefers a valid certificate over an expired one even when SkipValidation is set
  Expected strings to be the same, but they were different.
  Expected length: 25   (CN=Valid Test Certificate)
  Actual length:   27   (CN=Expired Test Certificate)

The mock deliberately returns the expired certificate first so the hoisted
version picks it. The hoist was reverted before the real fix was written.

The two SkipValidation-off tests and the two HasPrivateKey tests pass in both
directions; they pin behavior this PR must not change.

Test Plan

• ./build.ps1 -Task Test — 493 passed, 0 failed, 3 skipped. Baseline on
  main (a64ebc8) is 483/0/3; the ten added tests are the nine new
  SkipValidation cases plus the signing-settings comparison.
• Analyze (PSScriptAnalyzer) clean — the four surviving warnings are
  pre-existing, in New-PSBuildFileCatalog, Publish-PSBuildModule, and
  Test-PSBuildPester; none in the changed files.
• Red-before-green captured above for all five fixed behaviors, and against
  the naive-hoist alternative for the sixth.
• All touched files are CRLF throughout; no lone LF and no \r\r\n
  introduced (byte-counted).

README.md was not touched — but #190's row needs a follow-up

Open PR #190 documents Sign.SkipCertificateValidation as:

Skip the private key, expiration, and Code Signing EKU checks made on
certificates loaded by the EnvVar and PfxFile sources. Not recommended in
production.

That was accurate against the implemented behavior when it was written, and #193
explicitly noted that #190 would stay correct whichever way this issue was
resolved. Resolving it by fixing the code makes that no longer true: the row now
under-reports, telling a consumer the setting does nothing for Store and
Thumbprint when it does. Per instructions the row was not edited here — it
needs a follow-up, either a rewording commit on #190 before it merges or a small
docs PR after. #190's Settings documented in the README test compares setting
names, not descriptions, so nothing breaks in the meantime and the two PRs do
not conflict.

tests/IBTasks.tests.ps1 is also touched by #186 and #190, so the new Describe
block is appended at the end of the file and nothing already there was
restructured.

Breaking Changes

None in the strict sense — SkipCertificateValidation defaults to $false and
the behavior with it off is unchanged. Consumers who had it set to $true with a
Store or Thumbprint source were getting nothing from it before and now get
the documented fallback, which is the point of the fix.

🤖 Generated with Claude Code

https://claude.ai/code/session_01U1Jhu7fgTRJq7LK5MuKteE

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[github-actions]

Test Results

    4 files  ± 0    785 suites  +12   2m 42s ⏱️ +20s
  508 tests +10    505 ✅ +10   3 💤 ± 0  0 ❌ ±0 
2 015 runs  +40  1 921 ✅ +13  94 💤 +27  0 ❌ ±0 

Results for commit 1c1b059. ± Comparison against base commit 39cb303.

♻️ This comment has been updated with latest results.