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

Should Pkg.clone run Pkg.build? #12891

Closed
jiahao opened this issue Aug 31, 2015 · 29 comments
Closed

Should Pkg.clone run Pkg.build? #12891

jiahao opened this issue Aug 31, 2015 · 29 comments
Labels
decision A decision on this change is needed packages Package management and loading
Milestone

Comments

@jiahao
Copy link
Member

jiahao commented Aug 31, 2015

I just noticed that when Pkg.clone-ing an unregistered package, Pkg.build does not appear to be run automatically. In contrast, Pkg.add calls Pkg.build to build binary dependencies.

Is this an inconsistency between Pkg.add and Pkg.clone? This issue came up when I was trying out an unregistered package that required a binary external dependency and I was confused about why the call to the library it wrapped was failing.

@IainNZ
Copy link
Member

IainNZ commented Aug 31, 2015

IMO, it should. If I didn't want to run the build, it should maybe accept a keyword argument to stop that, but I've never not wanted to do the build. This can cause trouble when people are playing around on Travis too, I've observed.

@jiahao jiahao added decision A decision on this change is needed packages Package management and loading labels Aug 31, 2015
@quinnj
Copy link
Member

quinnj commented Aug 31, 2015

+1. I've wanted this several times as well (when TimeZones.jl was being developed, for example)

@malmaud
Copy link
Contributor

malmaud commented Aug 31, 2015

I can't think of a downside.

@stevengj
Copy link
Member

+1. Of course, Pkg.build may fail on an unregistered package that is a WIP, but clone should be tolerant of that.

@davidanthoff
Copy link
Contributor

+1

@wildart
Copy link
Member

wildart commented Sep 1, 2015

No, it shouldn't. Security and integrity is a major concern. I case of a registered package there is a bit of trust in a package, that it works correctly, because of developer's effort of putting the package into the public repository, METADATA, (which of course could be with a malicious intent). At least, there is a light review process for METADATA submission which could identify some package inconsistencies and problems. So, when build is called during the package installation, we could expect from it a proper behaviour (at least tested by developer).

clone command is a circumvent of a proper installation process that usually used while package is under development. Thus, nobody should expect from package correct behaviour until the release. So running build command that potentially executes incorrect code would constitute a big security problem. Moreover, if developer does not provide instruction about a correct package installation steps before it appears in METADATA, I would really think of using such package. Of course, if you want to tinker with a code, it is not a big deal to type one more command.

Bottom line of this rant: be careful of what you install and run on your machine, and value your security against convenience.

P.S. There was a discussion about the security build call. I could not find it.

@jiahao
Copy link
Member Author

jiahao commented Sep 1, 2015

I'm not sure how this security discussion is relevant. It's clear that quite a few developers want the convenience of having clone trigger build. Ordinary users shouldn't be clone-ing unregistered packages in the first place.

@tkelman
Copy link
Contributor

tkelman commented Sep 1, 2015

No. This is executing arbitrary code, that's not a good idea.

@stevengj
Copy link
Member

stevengj commented Sep 1, 2015

If you want to clone something without building, e.g. to look at the source code, you can always do git clone manually (and if you don't trust it, you probably shouldn't be sticking it in your .julia directory anyway). You use Pkg.clone if you are planning on installing and actually using the package, in which case you are going to be trusting its code anyway, so I don't see the additional risk of running Pkg.build.

@malmaud
Copy link
Contributor

malmaud commented Sep 1, 2015

What he said. It's not like build.jl from tagged packages on metadata are undergoing rigorous security audits anyways.

@tkelman
Copy link
Contributor

tkelman commented Sep 1, 2015

Rigorous security audits no, but part of metadata PR review should be looking over the package and checking for anything unusual.

This is like downloading a script and piping it directly to an interpreter without looking at it first. People do this all the time out of convenience, but that doesn't make it a good idea or something we should encourage. For code that hasn't been reviewed or looked at by anyone else, there should be a separation between downloading the code, running setup steps, and using it.

@sbromberger
Copy link
Contributor

FWIW, I'm with @wildart and @tkelman on this. If anything, have Pkg.clone() default to NO build unless there's an argument specified (that is, default to "no build" instead of defaulting to "build" as @IainNZ suggested). This way you get the benefit of having a simpler build step for cloned packages, but keep the safety of being able to examine experimental/untrusted code without executing arbitrary commands on your system.

@quinnj
Copy link
Member

quinnj commented Sep 1, 2015

Yeah, it's not terrible if you have to do Pkg.clone(url; build=true). For the most part, I'm giving them Pkg.clone(url) to run at the command line anyway.

@ScottPJones
Copy link
Contributor

I understand the convenience argument, but that reminds me of Windows, always choosing convenience over security, I'd also agree with @wildart and @tkelman on this.

@stevengj
Copy link
Member

stevengj commented Sep 1, 2015

"Security" does not mean "force users to type two commands instead of one". If a user is paranoid enough about a package to audit the code before running it, then they are a paranoid enough to type git clone. If they aren't paranoid enough to audit the code, forcing them to type two Pkg commands provides no additional security.

@sbromberger
Copy link
Contributor

With respect, this:

"Security" does not mean "force users to type two commands instead of one"

does not comport with this:

paranoid enough to type git clone

and in any event is incorrect, particularly since the Julia command is the same name as the git command, which would cause a reasonable person to assume they performed equivalent functions - that is, grab code. With this proposal, however, the former would then execute untrusted code.

Having an option to clone and build seems to be what people want, and there's no reason not to provide it; but repurposing a command that has historically been used to fetch only (and which has an analog in the underlying system it represents) to perform a potentially unsafe action that is currently present in neither implementation is just bad practice.

@tkelman
Copy link
Contributor

tkelman commented Sep 1, 2015

If we want to change the contract of what Pkg.clone does for an unregistered url, we might as well stop calling it clone.

@IainNZ
Copy link
Member

IainNZ commented Sep 1, 2015

Maybe Pkg.add should do clone and build if the argument is a URL?

@davidanthoff
Copy link
Contributor

+1 to having Pkg.add for git clone + Pkg.build when passed a URL.

@wildart
Copy link
Member

wildart commented Sep 2, 2015

Another issue that build will not be called for cloned packages on update which is not an issue with properly installed packages. So entering build command is still required.

@stevengj
Copy link
Member

stevengj commented Sep 2, 2015

Why even export a Pkg.clone command? Why not just use Pkg.add exclusively, and have it accept URLs? If users want a bare clone command why not just tell them to use git clone?

@malmaud
Copy link
Contributor

malmaud commented Sep 2, 2015

I have wondered the same thing

@wildart
Copy link
Member

wildart commented Sep 2, 2015

Why even export a Pkg.clone command? ... why not just ... use git clone?

for convenience?

@KristofferC
Copy link
Sponsor Member

With git clone you have to type the whole package name again without the .jl ending to get the correct folder name. Seconds wasted!!

@mschauer
Copy link
Contributor

mschauer commented Sep 2, 2015

A user with superificial knowledge of Pkg might be worried to use git clone in the .julia directory.
Unlike git clone the existence of Pkg.clone implies that this method of install in the package directory does not break some hidden contract about the layout of .julia and that the package system gracefully incorporates the Pkg.cloned repository in the julia system. (Using Pkg.add exclusively, and have it accept URLs would of course also be fine)

@davidanthoff
Copy link
Contributor

Does Pkg.clone do something related to the .cache subdirectory? I've never understood what that actually is, but it seems that if you do Pkg.clone it also puts another copy of the repo there, or something like that, and that is something a normal git clone wouldn't do.

@ScottPJones
Copy link
Contributor

I like the idea of using Pkg.add do clone & build with url, and having Pkg.clone just clone (to the right julia directories).

@tkelman
Copy link
Contributor

tkelman commented Sep 3, 2015

We also won't be bundling or depending on the presence of command-line git forever. @wildart's libgit2-based git repl mode will be usable for simple cases though.

@StefanKarpinski StefanKarpinski added this to the 0.6.0 milestone Sep 13, 2016
@tkelman
Copy link
Contributor

tkelman commented Jan 5, 2017

Closing this, will likely be reworked for Pkg3.

@tkelman tkelman closed this as completed Jan 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
decision A decision on this change is needed packages Package management and loading
Projects
None yet
Development

No branches or pull requests