Using pip as sudo

[yotabits]

Rosdep seems to be using pip as sudo.

 rosdep install --from-paths . --ignore-src -r -y --simulate --reinstall
#[pip] Installation commands:
  sudo -H pip install -U -I gTTS

I think this might be dangerous as pip packages are not reviewed.

[tfoote]

pip has moved to suggesting the installation into one's home directory recently. This is definitely something that we could consider supporting going forward. I think that we'd want to enable parameterizing the behavior so you can pick the target pip environment as a first step. Then we could let the user decide and potentially eventually change the default behavior if it's validated as working well enough.

[yotabits]

@tfoote For me, the issue about that is to run setup.py(which may contain malicious code.) as root.

From the other side, some packages need to be installed as sudo, it's not common, but it does happen.

Let's say pkg-a and pkg-b are both installed by pip pkg-a needs to be installed as a sudo, pkg-b don't. Considering all this we could modify https://github.com/ros/rosdistro/blob/master/rosdep/python.yaml like this:

pkg-a-pip-sudo:
   ubuntu:
         pip: pkg-a

pkg-b-pip:
   ubuntu:
         pip: pkg-b

This lets the user know that the dependency is installed as a sudo when he declares it in package.xml on the other hand rosdep will catch the *-pip-sudo and consequently run sudo pip

We can also change it like that:

pkg-a-pip:
   ubuntu:
      sudo_pip: pkg-a

pkg-b-pip:
   ubuntu:
         pip: pkg-b

This allows differentiating sudo installed dependencies from non sudo but will not:

• Let the user know that the dep is installed as sudo when he declares it in package.xml
• If we change a non sudo to a sudo (and vice versa) dependency in python.yaml the user will not be aware that the dependency installation method changed.

Another solution that allows to not pollute https://github.com/ros/rosdistro/blob/master/rosdep/python.yaml let's consider that src defines dependencies to pkg-a-pip and pkg-b-pip add options: --pip-as-sudo-all and --pip-as-sudo-for [DEPS]

 rosdep install --from-paths . --ignore-src -r -y --simulate --reinstall --pip-as-sudo-all
#[pip] Installation commands:
  sudo -H pip install -U -I pkg-a
  sudo -H pip install -U -I pkg-b

 rosdep install --from-paths . --ignore-src -r -y --simulate --reinstall --pip-as-sudo-for pkg-a-pip
#[pip] Installation commands:
  sudo -H pip install -U -I pkg-a
  pip install -U -I pkg-b

OTHERWISE BY DEFAULT
 rosdep install --from-paths . --ignore-src -r -y --simulate --reinstall
#[pip] Installation commands:
  pip install -U -I pkg-a
  pip install -U -I pkg-b

Please let me know if you have any other idea on how to solve this problem I would like to PR it if we find a consensus on how to solve that.

[tfoote]

I agree that there's a general risk here. We need to identify the precise scenarios that you're trying to avoid. In particular you're worried about the pip package being installed with sudo. Though presumably if the package author has declared the package as a dependency or the package has been added to the rosdep database someone has been using and testing with the package successfully enough. And as such we're mostly talking about a potential malicious person taking over the pip package uploading content. And to that end, it's much harder to do malicious things without sudo but certainly not impossible if their code is being run locally. I might suggest that for areas with specific security concerns that the pip installers be disabled overall. They can only be used for from source builds and the users could be instructed to manually resolve the dependencies listed.

An alternative approach might be to add a new pip-user installer that will install into the users's home directory instead of the system directory which is along the lines of your 2nd proposal. However, for backwards compatibility we should not change the existing behavior but add the desired behavior for the new implementation. This would improve the overall process but would require a transition period.

[yotabits]

Though presumably if the package author has declared the package as a dependency or the package has been added to the rosdep database someone has been using and testing with the package successfully enough. And as such we're mostly talking about a potential malicious person taking over the pip package uploading content.

Let's assume that I got fired from my company and I'm a very malicious person (no I'm not :rabbit:) I know their technology and codebase, one of their packages I wrote depends on a pip package I have released on Pypi and made available. I can make a lot of damage to the company if I decide to release a malicious version of my package and I can make absolutely transparent to them as packages.xml do not support versions.

An alternative approach might be to add a new pip-user installer that will install into the users's home directory instead of the system directory which is along the lines of your 2nd proposal. However, for backwards compatibility we should not change the existing behavior but add the desired behavior for the new implementation. This would improve the overall process but would require a transition period.

I totally agree, from the backward compatibility POV it's terrible, what do you think about the following options not creating any backward compatibility issue as far as I can figure out:

--disable-pip
--pip-as-current-user
--pip-as-user-for [DEPS]

[tfoote]

For turning on or off sudo globally I think you'll find we already have an option for that --as-root=pip:false or --as-root=pip:true

This could be extended to support package specific arguments too theoretically.

There is an option for filtering the installers when querying the database --filter-for-installers apt I think it's specific to the db verb but would probably be best to extend it's usage to support the other verbs.

[yotabits]

Thank you for the tips I actually didn't know those options exist.

There is an option for filtering the installers when querying the database --filter-for-installers apt I think it's specific to the db verb but would probably be best to extend it's usage to support the other verbs.

I'm not sure I understood what you mean by db verb or what you except, could you please give me an example or explain a bit more in details?

[tfoote]

I'm not sure I understood what you mean by db verb or what you except, could you please give me an example or explain a bit more in details?

You can call rosdep db which is COMMAND VERB so db is the verb. The option --filter-for-installers only applies to that code path. It could be extended to work for all the other verbs.

It's documented in rosdep -h:

  --filter-for-installers=FILTER_FOR_INSTALLERS
                        Affects the 'db' verb. If supplied, the output of the
                        'db' command is filtered to only list packages whose
                        installer is in the provided list. The option can be
                        supplied multiple times. A space separated list of
                        installers can also be passed as a string. Example:
                        `--filter-for-installers "apt pip"`