chocolatey / cChoco

Community resource to manage Chocolatey
Apache License 2.0
154 stars 99 forks source link

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

Closed artisticcheese closed 3 years ago

artisticcheese commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago

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 commented 3 years ago
  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 commented 3 years ago

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 commented 3 years ago

I think this is where the bug hits: https://github.com/chocolatey/cChoco/blob/feffeedcde2d93bcb570cb43ff483856c095ee88/DSCResources/cChocoInstaller/cChocoInstaller.psm1#L193

artisticcheese commented 3 years ago

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 commented 3 years ago

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?