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

cChocoInstaller fails to install Chocolatey, chocolatey folder already exists #151

Closed
artisticcheese opened this issue Feb 5, 2021 · 9 comments · Fixed by #153
Closed

Comments

@artisticcheese
Copy link

Describe the bug
Right now cChocoinstaller will download installation package and will execute it ignoring warnings from installation package and still adding environment variables etc to system

    Get-FileDownload -url $ChocoInstallScriptUrl -file $file
    . $file

Actual code for chocolatey install (below) will consider presence of folder where chocolatey is installed and presense any file there (which is where DSC downloads file) an indication that chocolatey is already installed. This will prevent chocolatey to be ever installed and lead to errors like below

function Test-ChocolateyInstalled {
    [CmdletBinding()]
    param()
    $checkPath = if ($env:ChocolateyInstall) { $env:ChocolateyInstall } else { 'C:\ProgramData\chocolatey' }
    if (Get-Command choco -CommandType Application -ErrorAction Ignore) {
        # choco is on the PATH, assume it's installed
        $true
    }
    elseif (-not (Test-Path $checkPath)) {
        # Install folder doesn't exist
        $false
    }
    elseif (-not (Get-ChildItem -Path $checkPath)) {
        # Install folder exists but is empty
        $false
    }
    else {
        # Install folder exists and is not empty
        $true
    }
}

Error

VERBOSE: [dominos-web]:                            [[cChocoInstaller]Install] Env:ChocolateyInstall has C:\Choco
VERBOSE: [dominos-web]:                            [[cChocoInstaller]Install] Downloading https://chocolatey.org/install.ps1 to C:\Choco\install.ps1
WARNING: [dominos-web]:                            [[cChocoInstaller]Install] An existing Chocolatey installation was detected. Installation will not continue.
For security reasons, this script will not overwrite existing installations.

Please use choco upgrade chocolatey to handle upgrades of Chocolatey itself.
VERBOSE: [dominos-web]:                            [[cChocoInstaller]Install] Adding Choco to path
VERBOSE: [dominos-web]:                            [[cChocoInstaller]Install] Env:Path has C:\windows\system32;C:\windows;C:\windows\System32\Wbem;C:\windows\System32\WindowsPowerShell\v1.0\;C:\windows\System32\OpenSSH\;C:\Choco
The term 'Choco' is not recognized as the name of a cmdlet, function, script file, or operable program. Check the spelling of the name, or if a path was included, verify that the path is correct and try again.
At C:\Packages\Plugins\Microsoft.Compute.CustomScriptExtension\1.10.9\Downloads\0\webserver\server.process.ps1:77 char:33
+ Start-DscConfiguration -path "$($env:temp)\prep" -wait -Verbose -erro ...
+                                 ~~~~~~~~~
    + CategoryInfo          : ObjectNotFound: (Choco:) [], CimException
    + FullyQualifiedErrorId : CommandNotFoundException
    + PSComputerName        : localhost

To Reproduce

   Node localhost {
      cChocoinstaller Install {
         InstallDir = "C:\Choco"

      }
}

Will fail
Expected behavior
Shall install chocolatey

@mattmcspirit
Copy link

I'm seeing the same behavior - cleaning things up, like the c:\choco directory, PATH variables, registry etc and just running the install from PowerShell directly on the same machine (without using DSC) seems to work fine.

Set-ExecutionPolicy Bypass -Scope Process -Force; [System.Net.ServicePointManager]::SecurityProtocol = [System.Net.ServicePointManager]::SecurityProtocol -bor 3072; iex ((New-Object System.Net.WebClient).DownloadString('https://chocolatey.org/install.ps1'))

All was working fine yesterday.

@ferventcoder
Copy link
Member

Note we pushed a security fix to install.ps1 plus quite a bit of cleanup. We've checked quite a bit with it, but it's hard to catch every area of things that could occur.

Here's where the problem lies:

VERBOSE: [dominos-web]: [[cChocoInstaller]Install] Env:ChocolateyInstall has C:\Choco
VERBOSE: [dominos-web]: [[cChocoInstaller]Install] Downloading https://chocolatey.org/install.ps1 to C:\Choco\install.ps1

That should be downloaded elsewhere, not to the folder where the installation is. So yeah, the install script will detect the folder is already created and not install there.

@ferventcoder
Copy link
Member

ferventcoder commented Feb 5, 2021

To insulate yourself from changes that we are making in terms of the community and the packages repository (as we will be making quite a few over the next few months), it would be best to provide a script to point to a chocolatey nupkg in your internal infrastructure.

This is also going to be your best workaround while this gets fixed in cChoco.

@artisticcheese
Copy link
Author

I think issue on both sides

  1. Chocolatey install script shall not consider non-empty folder as valid choco installation
  2. DSC resource shall not proceed with setting environment variables unless installation succeeded while ignoring Warning being thrown in process

@ferventcoder
Copy link
Member

ferventcoder commented Feb 5, 2021

  1. Chocolatey install script shall not consider non-empty folder as valid choco installation

The problem lies in that someone could pre-create the folder and drop a hijacking dll in here. Then when Chocolatey installs and puts folders on the SYSTEM path, those hijacking DLLs are still there. Even though it installs and asserts admin privileges to the default location (not to C:\Choco - you are left to handle those when you customize the path), it takes everything existing along for the ride.

So to fix the security issue, if there is an existing folder where Chocolatey was going to install, it just doesn't. Errorring in a terminating way on this is just completely out - that would cause a lot consternation as compared to what is currently being seen (it would affect a far larger number of users).

  1. DSC resource shall not proceed with setting environment variables unless installation succeeded while ignoring Warning being thrown in process

There are a few things to do in cChoco on this. I think we may see some small issues with other integrations as well.

I am sorry to hear we didn't catch this as part of our testing.

@artisticcheese
Copy link
Author

DSC resource has property ChocoInstallScriptURL, it would be good then to option to provide option to set in stone which version of install script resource will use so integration would not break when new versions are released (like what you can do with docker tags)

@steviecoaster
Copy link

I think this is where the bug hits:

$file = Join-Path -Path $InstallDir -ChildPath 'install.ps1'

@ferventcoder ferventcoder changed the title Installation shall not proceed if errors/warning shown in chocolatey package cChocoInstaller fails to install Chocolatey, chocolatey folder already exists Feb 6, 2021
@artisticcheese
Copy link
Author

In the interim you can use following to make cChoco resource work. It's official install.ps1 file which does not consider Choco folder with single file as a valid chocolatey installation

   cChocoinstaller Install {
         InstallDir            = "C:\Choco"
         ChocoInstallScriptUrl = "https://gist.githubusercontent.com/artisticcheese/d934c1fb704a3e67b3c68283bcabca66/raw/9345bcb115ee7350172fa00085514212245a1c65/install.ps1"

      }

@Artalus
Copy link

Artalus commented Feb 8, 2021

As @steviecoaster pointed, the problem is simply that cChoco downloads the install.ps1 into the very folder where chodolatey will reside. Can the folder simply be changed to %TMP%? Or perhaps, even provide an option as to where to download the script to?

Yvand added a commit to Yvand/AzureRM-Templates that referenced this issue Feb 8, 2021
pauby added a commit to pauby/cChoco that referenced this issue Feb 9, 2021
Changes in the Chocolatey install.ps1 means downloading to Choco install folder stops the install. Amended the code to download the install.ps1 file to a temporary folder and execute it from there.
@pauby pauby closed this as completed in #153 Feb 9, 2021
pauby added a commit that referenced this issue Feb 9, 2021
@pauby pauby added this to the 2.5.0 milestone Feb 9, 2021
pauby added a commit that referenced this issue Feb 9, 2021
* development:
  Update module versions for release
  (GH-151) Download install.ps1 to temp folder.
  (maint) Set pester to a max version of 4.10.1
  (GH-20) add cChocoConfig resource
  Adding comments and tests
  Add pull request template
  (doc) Add issue template configuration
  (doc) Remove invalid issue template
  (doc) Update issue templates
  throw if using prerelease with minimumversion
  quick fixes
  DSCResources/cChocoPackageInstall/cChocoPackageInstall.psm1
  adding support for specifying minimum version
Yvand added a commit to Yvand/AzureRM-Templates that referenced this issue Feb 15, 2021
* Workaround to cChco issue chocolatey/cChoco#151

* Implement temporary workaround to blocking cChoco issue

* Update DSC module cChoco to 2.5

* Update DSC module cChoco to 2.5

* Fix installation of Fiddler

* Revert "Fix installation of Fiddler"

This reverts commit cac5bf1.
jahmai-ca added a commit to criticalarc/sz-azure-deploy that referenced this issue Jul 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants