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.

Sign up for GitHub

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

Jump to bottom

(git.install) Terminate gpg-agent when indirectly upgrading #1945

Merged
merged 2 commits into from
Aug 8, 2022
Merged

(git.install) Terminate gpg-agent when indirectly upgrading #1945

merged 2 commits into from
Aug 8, 2022

Conversation

brogers5
Copy link
Contributor

@brogers5 brogers5 commented Jul 6, 2022

Description

This changeset implements a package-level workaround for chocolatey/choco#1092, as originally discussed in #1924.

Fixes #1938.

Motivation and Context

When upgrading git.install indirectly (via either choco upgrade git or choco upgrade all, which will alphabetically iterate through git first), choco will fail to execute chocolateyBeforeModify.ps1 of any package dependencies being installed. In the context of git.install, this contains commands to stop any running instances of gpg-agent.exe, which may run as a background process when commit signing is enabled. Commit signing is a commonly enabled feature to help certify the authenticity of a given commit's author.

While this would ideally be fixed upstream in choco, there is currently no ETA on when a fix would roll out. As a stopgap solution, wrapping this functionality within a new function defined in helpers.ps1, and calling the function from both chocolateyBeforeModify.ps1 and chocolateyInstall.ps1. The former is required to accommodate uninstall commands, while the latter should cover any upgrade scenario, whether invoked directly (via choco upgrade git.install) or indirectly.

How Has this Been Tested?

Setup

  1. Manually download Git for Windows installer binaries to git.install's tools directory, as they are not committed to this repository.
  2. choco pack within both git and git.install directories of local repository to create test packages with proposed changeset.
  3. Copy test packages to Chocolatey Test Environment VM. All steps from this point forward involve the VM.
  4. Install previous version (assumed 2.36.1) of git and git.install packages with choco install git --version=2.36.1 --params "/GitAndUnixToolsOnPath".
  5. Confirm install command completes successfully.
  6. Reopen PowerShell.
  7. Create a GPG signing key with gpg --full-generate-key. Default options were used, with arbitrary user ID and passphrase.
  8. Start gpg-agent.exe with gpgconf --launch gpg-agent.
  9. Open Task Manager and confirm gpg-agent.exe has launched.

Test Cases

Indirect Package Upgrade via Metapackage

  1. Upgrade package to test package version with choco upgrade git --source="." (assuming packages are in current directory).
  2. Confirm gpg-agent.exe is successfully closed, and the upgrade command completes without errors.

Direct Package Upgrade

  1. Restart gpg-agent.exe with gpgconf --launch gpg-agent.
  2. Force a direct package upgrade with choco upgrade git.install --source="." --force (assuming package is in current directory).
  3. Confirm gpg-agent.exe is successfully closed, and the upgrade command completes without errors.

Uninstall

  1. Restart gpg-agent.exe with gpgconf --launch gpg-agent.
  2. Uninstall the test package version with choco uninstall git.
  3. At runtime, confirm uninstallation of git.install package as well.
  4. Confirm gpg-agent.exe is successfully closed, and the uninstall command completes without errors.

Screenshot (if appropriate, usually isn't needed):

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Migrated package (a package has been migrated from another repository)

Checklist:

  • My code follows the code style of this repository.
  • My change requires a change to documentation (this usually means the notes in the description of a package).
  • I have updated the documentation accordingly (this usually means the notes in the description of a package).
  • I have updated the package description and it is less than 4000 characters.
  • All files are up to date with the latest Contributing Guidelines
  • The added/modified package passed install/uninstall in the chocolatey test environment.
  • The changes only affect a single package (not including meta package).

@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

@brogers5
(git.install) Address pull request feedback, some additional related …
bf05f11 
…fixes and optimizations

* Use -like comparison operator instead of -ilike
* Update Install-ChocolateyInstallPackage's SoftwareName parameter to currently used name to correctly detect install location
* Remove return statement from installLocation check. This prevented code execution beyond it (notably failing to honor the NoCredentialManager package parameter), and is not a critical failure
* Refactor stop process helper functions to a parameterized implementation
* Optimize process lookup to a single Get-Process invocation per process search
* Implement process filtering based on detected install location instead of implied Git directory
@brogers5
Copy link
Contributor Author

brogers5 commented Aug 6, 2022

New commit pushed with changes to address feedback and implement some additional fixes.

Additional tests ran with a newly built version:

Enhancement: gpg-agent Termination Tweaks

Context: As previously noted, the installation directory can be overridden, by both FOSS users (using installer argument /DIR="value") or Pro+ users (using choco's Ubiquitous Install Directory option [--install-directory="value"]). The process filtering should work as follows:

  • If no instances of gpg-agent.exe are running, skip termination.
  • If we cannot find the install location of Git, assume it is not installed and skip termination.
  • We should account for possible install directory redirection, and filter detected instances by Git's detected install location.

Test Case: No Running gpg-agent Instance

  1. Install test package version: choco install git.install --source="."
  2. Open Task Manager and confirm no instances of gpg-agent.exe are running.
  3. Uninstall the package with choco uninstall git.install (assuming meta-package is not currently installed).
  4. Confirm Killing any running git gpg-agent instances is NOT printed.
  5. Confirm the uninstall command completes without errors.

Test Case: Undetected Install Location

  1. Install test package version: choco install git.install --source="."
  2. Uninstall Git outside of Chocolatey.
  3. Uninstall the package with choco uninstall git.install (assuming meta-package is not currently installed).
  4. Confirm Killing any running git gpg-agent instances is NOT printed.
  5. Confirm the uninstall command completes without errors.

Test Case: No Running Git gpg-agent Instance

  1. Install GnuPG with choco install gnupg.

  2. Reopen PowerShell.

  3. Start GnuPG's gpg-agent.exe with gpgconf --launch gpg-agent.

  4. Open Task Manager and confirm GnuPG's gpg-agent.exe has launched.

  5. Install test package version: choco install git.install --source="."

    NOTE: Stop-GitGPGAgent will also be executed during this to accommodate indirect upgrade cases. Uninstalling Git for Windows may orphan a gitconfig file within etc of the install directory, which could cause Get-AppInstallLocation to return an install location, regardless of whether Git is currently installed. This may cause gpg-agent termination to be attempted here.

  6. Uninstall the package again with choco uninstall git.install (assuming meta-package is not currently installed).

  7. Confirm Killing any running git gpg-agent instances is printed.

  8. Confirm GnuPG's gpg-agent.exe is NOT closed.

  9. Confirm the uninstall command completes without errors.

Test Case: Co-existing gpg-agent Instances

  1. Install test package version with choco install git.install --source=".".
  2. Start Git Bash.
  3. In Git Bash, start Git's gpg-agent.exe with gpg-agent -v --daemon.
  4. Open Task Manager and confirm Git's gpg-agent.exe has launched.
  5. Open Task Manager and confirm GnuPG's gpg-agent.exe is still running. If needed, relaunch it.
  6. Close Git Bash.
  7. In PowerShell, uninstall the package with choco uninstall git.install (assuming meta-package is not currently installed).
  8. Confirm Killing any running git gpg-agent instances is printed.
  9. Confirm Git's gpg-agent.exe is successfully closed.
  10. Confirm GnuPG's gpg-agent.exe is NOT closed.
  11. Confirm the uninstall command completes without errors.

Test Case: Redirected Install Location

  1. Reinstall test package version with an overridden install directory: choco install git.install --source="." --install-arguments="/DIR=""C:\Git""".
  2. Start Git Bash.
  3. In Git Bash, start Git's gpg-agent.exe with gpg-agent -v --daemon.
  4. Open Task Manager and confirm Git's gpg-agent.exe has launched.
  5. Close Git Bash.
  6. In PowerShell, uninstall the package with choco uninstall git.install (assuming meta-package is not currently installed).
  7. Confirm Killing any running git gpg-agent instances is printed.
  8. Confirm Git's gpg-agent.exe is successfully closed.
  9. Confirm the uninstall command completes without errors.

Regression Test: ssh-agent Termination

Context: I opted to deduplicate code between Stop-GitSSHAgent and Stop-GitGPGAgent, which now use a common parameterized implementation. While Stop-GitGPGAgent was also tested and likely provides sufficient test coverage for the core implementation, we should at least smoke test Stop-GitSSHAgent for reassurance.

  1. Install test package version with tools on path using choco install git.install --source="." --params "/GitAndUnixToolsOnPath".
  2. Reopen PowerShell.
  3. Start Git's ssh-agent using start-ssh-agent.cmd.
  4. Open Task Manager and confirm Git's ssh-agent.exe has launched.
  5. Force a direct package upgrade with choco upgrade git.install --source="." --force.
  6. Confirm Killing any running git ssh-agent instances is printed.
  7. Confirm Git's ssh-agent.exe is successfully closed.
  8. Confirm the upgrade command completes without errors.

Bug Fix: Install Location Detection

Context: chocolateyInstall.ps1's SoftwareName parameter for Install-ChocolateyInstallPackage was outdated. v2.33.0 changed the software's DisplayName property to Git instead of Git version *, and this broke the pattern search being done to detect this. I came across this since I also used Get-AppInstallLocation in the revised code.

  1. Install the test package version with choco install git.install --source=".".
  2. When installation completes, review console output.
  3. Confirm these warnings are NOT printed:
    • WARNING: No registry key found based on 'Git'
    • WARNING: Can't find git.install install location
  4. Confirm git.install installed to 'C:\Program Files\Git' is printed (assuming install directory was not overridden).
  5. Uninstall package with choco uninstall git.install (assuming meta-package is not currently installed).
  6. Reinstall test package version with an overridden install directory: choco install git.install --source="." --install-arguments="/DIR=""C:\Git""".
  7. When installation completes, review console output, and confirm printed install location is consistent with user-defined directory.

Bug Fix: /NoCredentialManager package parameter not being honored

Context: The install location detection bug unmasked this behavior. Previously, if we failed to detect the install location, we return out of the script entirely. This did not give the install script a chance to evaluate the /NoCredentialManager package parameter.

  1. Install the test package version with choco install git.install --source="." --params="/NoCredentialManager"
  2. When installation completes, review console output.
  3. Confirm Git credential manager will be disabled. was printed.
  4. Reopen PowerShell.
  5. Print the GCM_VALIDATE user environment variable with $env:GCM_VALIDATE.
  6. Confirm false was printed.

@brogers5 brogers5 requested a review from AdmiringWorm August 6, 2022 20:14
@AppVeyorBot
Copy link

✅ Package verification completed without issues. PR is now pending human review

AdmiringWorm
AdmiringWorm approved these changes Aug 8, 2022
View reviewed changes
Copy link
Member

@AdmiringWorm AdmiringWorm left a comment

Choose a reason for hiding this comment

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

LGTM

@AdmiringWorm AdmiringWorm merged commit 7a16603 into chocolatey-community:master Aug 8, 2022
@AdmiringWorm
Copy link
Member

@brogers5 your changes have been merged, thanks for your contribution 👍

@brogers5 brogers5 deleted the git.install-fix-gpg-agent-termination branch August 8, 2022 13:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AdmiringWorm AdmiringWorm AdmiringWorm approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

(git.install) Terminate gpg-agent as part of install?
3 participants
@brogers5 @AppVeyorBot @AdmiringWorm