-
Notifications
You must be signed in to change notification settings - Fork 108
feat(install.sh): sudo {mkdir|cp} if necessary #432
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
Conversation
This commit causes fpm to invoke mkdir and/or cp using sudo if the user does not have write permissions to the installation destination. Invoking sudo potentially prompts the user for a password.
My understanding of best practices with regards to |
@LKedward I've always been advised to do the exact opposite. Think of it this way: Moreover, running I strongly advise against recommending |
Interesting discussion! |
Thanks for the explanation Damian @rouson, I agree with your concerns about elevating the entire fpm binary. Your point is particularly acute for fpm currently since it relies heavily on With that said I am still unsure about the chosen methodology here to invoke |
How about this strategy:
Instead of doing sudo underneath, report the problem to the user and write
a small shell script/batch file that the user can run using sudo
him/herself.
Op do 8 apr. 2021 om 13:03 schreef Laurence Kedward <
***@***.***>:
… Thanks for the explanation Damian @rouson <https://github.com/rouson>, I
agree with your concerns about elevating the entire *fpm* binary. Your
point is particularly acute for *fpm* currently since it relies heavily
on execute_command_line for much of it's file-system access (see also #166
<#166>); this is something that
we aim to phase-out completely with a proper file-system library (*e.g.*
MarDiehl/stdlib_os <https://github.com/MarDiehl/stdlib_os>).
With that said I am still unsure about the chosen methodology here to
invoke sudo from within *fpm*. We can't use sudo if we want to switch to
using a file-system API call in future. I am no expert here, particularly
for cross-platform implementation, but I think a better approach would be
to use system APIs to drop any elevated permissions immediately when *fpm*
is started and then switch back only where needed. In POSIX, I think this
can be achieved with getuid/seteuid *etc.*
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#432 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAN6YR55NJGDK7QTHUTNCWLTHWEPPANCNFSM42SC4C3Q>
.
|
You should seldom run into a scenario where you have to use sudo in a build system. I have packaged for Arch Linux, msys2, brew and conda so far, Arch's makepkg even refuses to run under sudo. Usually a package manager will package in a local prefix instead of the system root, installing directly from the build system (good old |
@awvwgk Your comment makes sense for individual users but presumably not for system administrators. Is the intention that The whole point of the virtual machine is to ensure that course attendees and clients have necessary software pre-installed so that we don't spend time on set-up. This necessitates installing to a common space. Also, the default install locations for many (most?) CMake packages is in a subdirectory of |
I'd think it makes also (especially?) sense for system operators. I'm not familar with the dpkg / apt-get / .deb family of package managers, since I'm usually work with Arch Linux based distros. While the toolchains are named differently, I would expect the workflow to be similar:
Usually, this is done by somebody else and you can just access the binary package by |
I see merits to both sides of the argument. I still prefer fpm not doing anything internally about permissions, if at all possible. It's not fpm's responsibility and it adds complexity to future development. If you really need to install to system paths, then sudo it at your own risk. So I prefer this be handled with a note in the docs a-la:
|
We already have means to restrict fpm when running with elevated permissions, e.g. the install command supports a
Alternatively, there is always the do-it-yourself
|
I think this idea has merit. The security "principle of least privilege" says that elevated permissions should only be given when necessary. The whole fpm executable doesn't need escalated privileges, even for the command That said, we're not quite ready to do it in a cross-platform way reliably. We should create an issue to do this once we have a more fully featured |
Maybe relevant to this discussion: |
@awvwgk your latest comment is very helpful. I like the In retrospect, it would have been better for me to describe my use case in an issue before submitting this pull request. I was attempting to install |
This commit causes
fpm
to invokemkdir
and/orcp
usingsudo
ifthe user lacks write permissions to the installation destination on Unix-like
systems. Invoking
sudo
potentially prompts the user for apassword.