Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(#149) Switch to only signing when required #150

Merged
merged 1 commit into from
May 16, 2024

Conversation

gep13
Copy link
Member

@gep13 gep13 commented May 14, 2024

Description Of Changes

This commit addresses this need by changing the DAG to use a new Verify-PowerShellScipts task, rather than the Sign-PowerShellScripts task. The latter is still available to call directly, when required, but only when a valid certificate is in place.

Supporting parameters and build directories have been created, to allow control over what the tasks due, including the ability to skip the verification step, using the --shouldVerifyPowerShellScripts command line argument.

A new verify-powershell.ps1 file has been added to check the list of incoming files, and the sign-powershell.ps1 has been updated to only sign when the current signature is invalid. To aid with getting the signed files added to back into the repository, the signed files are uploaded as artifacts of the build.

Motivation and Context

We don't want to sign files when we don't need to. Going forward, PowerShell scripts are going to be signed when they are committed to the repository and only re-signed when required.

Testing

This will be a tricky one to test 😢

This will need to be tested in conjunction with this PR, and also in conjunction with a new build configuration for calling the updated Sign-PowerShellScripts task.

Happy to jump on a quick call to discuss further.

Operating Systems Testing

N/A

Change Types Made

  • Bug fix (non-breaking change).
  • Feature / Enhancement (non-breaking change).
  • Breaking change (fix or feature that could cause existing functionality to change).
  • Documentation changes.
  • PowerShell code changes.

Change Checklist

  • Requires a change to the documentation.
  • Documentation has been updated.
  • Tests to cover my changes, have been added.
  • All new and existing tests passed?
  • PowerShell code changes: PowerShell v2 compatibility checked?

Related Issue

Fixes #149

We don't want to sign files when we don't need to.  Going forward,
PowerShell scripts are going to be signed when they are committed to
the repository and only re-signed when required.

This commit addresses this need by changing the DAG to use a new
Verify-PowerShellScipts task, rather than the Sign-PowerShellScripts
task.  The latter is still available to call directly, when required,
but only when a valid certificate is in place.

Supporting parameters and build directories have been created, to allow
control over what the tasks due, including the ability to skip the
verification step, using the --shouldVerifyPowerShellScripts command
line argument.

A new verify-powershell.ps1 file has been added to check the list of
incoming files, and the sign-powershell.ps1 has been updated to only
sign when the current signature is invalid.  To aid with getting the
signed files added to back into the repository, the signed files are
uploaded as artifacts of the build.
@gep13 gep13 requested a review from Windos May 14, 2024 13:16
Copy link
Member

@Windos Windos left a comment

Choose a reason for hiding this comment

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

I've tested this as best I can, and all is looking good. Will wait until chatting with you before merging or any next steps.

Amongst the testing I did, I validated the logic in both the verify-powershell.ps1 and sign-powershell.ps1 scripts using the alternative signing certificate.

Initially, no scripts had a valid certificate. This is seen in the output of the verify script and the fact that it threw at the end of testing all scripts.

Verify, none valid

Next I manually signed three scripts, and the verify noted that these were now valid, but the rest weren't, and it still threw at the end as expected.

Verify, three valid

Next I ran the signing script, it saw that the three scripts I manually signed were already valid and signed the other 38 scripts. It copied only the scripts that it signed into the output directory as expected.

Sign, three pre-signed

Next, the signing script was re-run. Everything at this point was valid so nothing needed signing and therefore nothing was copied to the output directory.

Sign, all pre-signed

Finally, rerunning the verify script shows all signatures are now valid and it did not throw.

Verify, all valid

@Windos
Copy link
Member

Windos commented May 16, 2024

Thanks for getting this in, @gep13!

@Windos Windos merged commit e772355 into chocolatey:develop May 16, 2024
@gep13 gep13 deleted the verify-scripts branch May 16, 2024 07:09
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.

Switch to only signing PowerShell scripts when required
2 participants