psake / psake/PowerShellBuild

Invoke-PSBuildModuleSigning has no test that calls it

Open
#217 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

bug
Dominant language
PowerShell
Stars
145
Forks
27
Avg merge
10h 16m
Merged PRs (30d)
34

Description

tests/Invoke-PSBuildModuleSigning.tests.ps1 has eleven passing tests and none of them call Invoke-PSBuildModuleSigning. The function's body has never executed.

What happens

Two tests are named as though they exercise the function's file discovery:

It 'Searches for files matching Include patterns' {
    ...
    Mock Set-AuthenticodeSignature { [PSCustomObject]@{ Status = 'Valid'; Path = $InputObject } }

    # We need to skip this test if we can't create a real cert, or just verify file discovery
    # Instead of mocking cert, just count the files that would be signed
    $files = Get-ChildItem -Path $testDir -Recurse -Include '*.psd1', '*.psm1', '*.ps1'
    $files.Count | Should -Be 3  # Should not include .txt file
}

The Get-ChildItem call is in the test body. It asserts that Get-ChildItem filters by -Include, which is a fact about PowerShell, not about this module. Uses custom Include patterns when specified does the same thing with a one-element pattern list.

The Mock Set-AuthenticodeSignature in the first of the two is declared and then never used. It is also unscoped — no -ModuleName 'PowerShellBuild' — so it could not have reached the call inside the function even if the function had been called. That is the same defect as the six mocks in the sibling certificate test file.

The remaining nine tests inspect Get-Command metadata: that the command is exported, that its help has a synopsis and an example, that Path and Certificate are mandatory, that HashAlgorithm has the expected ValidateSet. Useful, but none of them run the function.

Measured

Code coverage over Invoke-PSBuildModuleSigning.ps1 with the whole file running:

11 tests passed, 0 failed
5 of 17 commands covered

All five covered commands are the ValidateScript on the Path parameter, which runs during parameter binding — reached by Validates that Path must be a directory, the one test that does bind against the command:

Line  Command
  60  if (-not (Test-Path -Path $_ -PathType Container)) { throw ... }
  60  Test-Path -Path $_ -PathType Container
  61  throw ($LocalizedData.PathArgumentMustBeAFolder)

Everything past the param() block is unexecuted:

$files = Get-ChildItem -Path $Path -Recurse -Include $Include
Write-Verbose ($LocalizedData.SigningModuleFiles -f $files.Count, ($Include -join ', '), $Path)

$sigParams = @{
    Certificate     = $Certificate
    TimestampServer = $TimestampServer
    HashAlgorithm   = $HashAlgorithm
}

$files | Set-AuthenticodeSignature @sigParams

Why it matters

This is the function that signs a consumer's shipped module. Nothing currently checks that:

  • the Include patterns the consumer configured are the ones passed to Get-ChildItem, or that -Recurse is passed at all;
  • TimestampServer and HashAlgorithm reach Set-AuthenticodeSignature rather than being accepted and dropped — the tests assert only that the parameters exist;
  • $Certificate is the certificate used;
  • every discovered file is signed, rather than only the first.

Any of those could be broken by a refactor with the suite staying green. $PSBPreference.Sign.HashAlgorithm could silently stop being honoured the same way $PSBPreference.Sign.SkipCertificateValidation silently stopped being honoured in #193, and there is no test here that would notice.

The gap is easy to close, because Set-AuthenticodeSignature is the only thing in the body that touches the outside world. Mocked with -ModuleName 'PowerShellBuild', the function can be driven end to end against files in $TestDrive with no certificate and no signing.

Options

  1. Call the function with a mocked Set-AuthenticodeSignature. A PSCustomObject stands in for the certificate, since the parameter is typed X509Certificate2 — which needs either a real certificate or a small [X509Certificate2]::new() from generated bytes to satisfy the binder. Then Should -Invoke -ModuleName 'PowerShellBuild' Set-AuthenticodeSignature -Times 3 -Exactly with a -ParameterFilter on TimestampServer and HashAlgorithm covers the whole body and every forwarding question above. This is the shape the retry-loop tests in Test-PSBuildScriptAnalysis.tests.ps1 already use, and they are the strongest tests in the suite.
  2. Rewrite the two misnamed tests rather than adding to them. Searches for files matching Include patterns and Uses custom Include patterns when specified should either call the function or be deleted; leaving them as they are means the file reports coverage of behaviour it does not have.
  3. Remove the unused Mock Set-AuthenticodeSignature, or scope it and use it. As written it reads like protection against accidentally signing something during a test run, and it is not.

(1) subsumes (2) and (3). The Certificate parameter's type is the only real obstacle, and a self-signed certificate generated into $TestDrive or a PSCustomObject cast through a looser parameter type both solve it.

Related: #103 (Publish-PSBuildModule had the same zero-coverage shape and the same consequence — a real defect, #203, sat undetected behind passing tests). Shares its mock-scoping defect and its remediation with #216, which has the same unscoped Mock problem in tests/Get-PSBuildCertificate.tests.ps1.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with tests/Invoke-PSBuildModuleSigning.tests.ps1 and the Invoke-PSBuildModuleSigning.ps1 function body. Replace or revise the two file-discovery tests so they call the function, using the scoped Set-AuthenticodeSignature mock pattern described in the issue. Run the test file and verify coverage includes discovery, parameter forwarding, and signing every discovered file.

Written by the indexing model from the issue text.

Assessment

Tech stack
powershell
Domain
testing-qa
Issue type
Refactor
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
76/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.