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

Do not print PowerShell install/update scripts by default #182

Closed
christianrondeau opened this issue Mar 20, 2015 · 8 comments
Closed

Do not print PowerShell install/update scripts by default #182

christianrondeau opened this issue Mar 20, 2015 · 8 comments

Comments

@christianrondeau
Copy link
Contributor

When running chocolatey without the -y flag, the script that will be run is printed by default. I suggest not doing that.

I believe that most users will skip them or trust them, regardless of whether they are printed or not. Also, it creates a very long output that is harder to read. Finally, the user is still running a script that is intended to run another executable and modify the system.

Here are some options I believe would allow more advanced users to still take advantage of this feature:

  • When asking whether the user wants to run the script or not, add an option to print it (e.g. Yes/No/Skip/Print)
  • Add a global setting to enable script printing

For more information: https://groups.google.com/forum/#!topic/chocolatey/UVncL7PxXRg

(Moved from chocolatey-archive/chocolatey#690)

Note: Similar to #178 but rather than suggesting confirmation is not needed, I'm here suggesting that we don't print by default; #53 (Yes to all) and the global settings are sufficient IMHO.

@ferventcoder
Copy link
Member

We'll want to split the non-moderated piece, it will be until we have package signing before we can add that.

@christianrondeau
Copy link
Contributor Author

Removed the part that said "Flag non-moderated scripts (a warning saying the user should print)"

@christianrondeau
Copy link
Contributor Author

I have a first implementation ready to go. Here's what the output looks like:

kitty.portable v0.63.2.2
The package kitty.portable wants to run 'chocolateyInstall.ps1'.
Note: If you don't run this script, the installation will fail.
Do you want to run the script?
 1) yes
 2) no [Default - Press Enter]
 3) print
3
------ BEGIN SCRIPT ------

# note: we cannot find public url to download from, so download them from http://www.9bis.net/kitty/?page=Download and put them in tools
#$scriptPath = $(Split-Path -parent $MyInvocation.MyCommand.Definition)
#$fileFullPath = Join-Path $scriptPath 'kitty.exe'
#Get-ChocolateyWebFile 'kitty' "$fileFullPath" 'http://www.fosshub.com/download/kitty.exe'

# note: already called by default
#$installDir = $(Split-Path -parent $MyInvocation.MyCommand.Definition)
#Get-ChocolateyBins $installDir


------- END SCRIPT -------
Do you want to run this script?
 1) yes
 2) no [Default - Press Enter]

Here's what's new:

If everything is good, I will issue the PR (click here to see the code changes). You'll notice that again there is no new tests, since it only changes the output and console prompt, and I could not find tests that verified that (related to that discussion... since most of my PRs for now are related to console output, I only verify that all tests are green and test manually in a console)

@ferventcoder
Copy link
Member

Like it.

ferventcoder added a commit that referenced this issue Apr 14, 2015
* 182-no-ps1-print:
  (GH-182) Ask before printing ps1 scripts
@ferventcoder
Copy link
Member

Merged into stable at ccd3c65 and will be released in 0.9.9.5

@ferventcoder ferventcoder added this to the 0.9.9.5 milestone Apr 17, 2015
ferventcoder added a commit that referenced this issue Apr 20, 2015
* stable: (22 commits)
  (GH-121) Making Uninstall-ChocolateyZipPackage more robust when
deleting files that were copied during installation of the Zip package
  (doc) update changelog/nuspec
  (GH-238) ApiKey source matching intuitive
  (maint) formatting
  (GH-240) Set CredentialProvider for NuGet
  (GH-240) ChocolateyNugetCredentialProvider
  (GH-240) Add default sources to machine sources
  (maint) Only warn subcommand list if not empty
  (GH-171) Use RedirectedHttpClient
  (GH-240) pass credentials at runtime
  (GH-240)(config) Add machine sources
  (doc) how to quote values
  (GH-230) Export all functions and aliases imported
  (GH-230) Fix Issues with Generate/Remove BinFile
  (GH-185) Remove console prompt default choices
  (GH-186) Uninstall - no prompt for one version
  (GH-182) Ask before printing ps1 scripts
  (GH-187) Show log file path in messages.
  (maint) formatting
  (GH-169) Do not resolve disabled sources
  ...

Conflicts:
	src/chocolatey/infrastructure.app/commands/ChocolateySourceCommand.cs
	src/chocolatey/infrastructure.app/runners/GenericRunner.cs
@ivanatpr
Copy link

ivanatpr commented Jun 5, 2015

This change seems misguided to me, reducing the script confirmations to something just shy of security theater. With this change, the interface now suggests that the normal behavior is to make a decision without looking at the script, which makes the default behavior of asking for those confirmations in the first place seem silly. You're preserving almost all the pain of the confirmations (which is flow-stopping prompt itself, not the messy output) while greatly reducing their main benefit (letting people know exactly what's being run and, more importantly, putting as many eyeballs as possible on each install script making it far more likely that the community will catch any malicious or buggy scripts that slip through moderation).

To put it another way, in my mind, this is what the new flow basically amounts to:

User: Install this app
Choco: In order to install something I gotta run an installation script. Crazy, right? Should I go ahead and do exactly what you just told me to do?

Meanwhile, the old way at least made a bit of sense:

User: Install this app
Choco: In order to install that app I gotta run the following script:
<script contents printed here.>
<Observant technical users can plainly see that chocolatey isn't just running the app's installer directly or some tightly constrained DSL but rather that they've actually just downloaded a random script that they're about to run in an elevated shell. Paranoia Intensifies.>

@ferventcoder
Copy link
Member

@ivanatpr I agree and disagree. Right now some of these are a wall of text (which makes some folks eyes gloss over). What I think we will move towards is an even better way to inspect package contents (as in open package folder and let me see it), so that a user can make a decision on whether they want to run the scripts or not.

@christianrondeau
Copy link
Contributor Author

@ivanatpr My thoughts, since I participated to this:

  • Even though we print the PowerShell script, we cannot verify that the installer executable/msi is safe, or that the script doesn't run another malicious script
  • Not everyone can understand PowerShell at all, so the feature will be seen as "noise" for some
  • Some scripts are quite long, and without color coding, it's hard to see malicious code (especially if someone wants to hide something, or when running cup all)
  • Running choco with the -y flag or running them in an automated script (widely used, e.g. boxstarter) will hide the scripts anyway
  • Anyone who cares about what runs on their machine can still easily choose to print scripts one at a time
  • There is a moderation process being put in place, which should increase the trust in chocolatey packages (at least as much as the installer itself)
  • No other package manager (at least that I know about) does that, including apt-get or even npm (even though that last one does not run with administrative privileges)

I personally think your point is still valid, and I do like @ferventcoder's idea of allowing the user to open and inspect the install script, the installer file and anything else it downloads on your machine. There could be other improvements, but I guess the whole point of chocolatey is to make it easy and straightforward to install software on your machine...

I'd be curious to know how other package managers deal with that issue.

@ferventcoder ferventcoder self-assigned this Jan 29, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants