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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --askpass argument to 'sudo' if it is not an interactive terminal #3727

Merged
merged 4 commits into from Oct 15, 2018

Conversation

Projects
None yet
5 participants
@jgsogo
Copy link
Member

commented Oct 11, 2018

Changelog: Fix: Add --askpass argument to sudo if it is not an interactive terminal

I'm supposing that everywhere sudo is used, the argument --askpassp is available. I hope this statement to be true 馃

@ghost ghost assigned jgsogo Oct 11, 2018

@ghost ghost added the stage: review label Oct 11, 2018

@jgsogo jgsogo requested a review from lasote Oct 11, 2018

@ArekPiekarz

This comment has been minimized.

Copy link

commented Oct 11, 2018

What happens when we start the noninteractive terminal as root and run conan in a way that triggers installing system packages? Without this commit, would it succeed without asking for the password? With this commit, would --askpass unnecessarily block the execution until a password is provided? Or would it be successful anyway?

@lasote

This comment has been minimized.

Copy link
Contributor

commented Oct 15, 2018

I think in that case "sudo" won't require a password so the --askpass argument won't have any effect, but please, @jgsogo can you try?

@jgsogo

This comment has been minimized.

Copy link
Member Author

commented Oct 15, 2018

On docker lasote/conantests, given

from conans.client.tools.system_pm import SystemPackageTool

sp = SystemPackageTool()

Use cases:

  1. No-root, tty:

    >>> sp.install("boost-dev")
    dpkg-query: no packages found matching boost-dev
    Running: sudo apt-get update
    [sudo] password for tom:
    
  2. No-root, no-tty:

    >>> sp._tool._sudo_str = "sudo --askpass "
    >>> sp.install("boost-dev")
    dpkg-query: no packages found matching boost-dev
    Running: sudo --askpass apt-get install -y --no-install-recommends boost-dev
    sudo: no askpass program specified, try setting SUDO_ASKPASS
    Traceback (most recent call last):
    
  3. No-root, no-tty, with SUDO_ASKPASS
    With file pw.sh:

    #!/bin/bash
    echo 'invalid-root-pass'
    

    and running python with the env variable

    $ SUDO_ASKPASS=/home/conan/conan/pw.sh python
    
    
    >>> sp._tool._sudo_str = "sudo --askpass "
    >>> sp.install("boost-dev")
    dpkg-query: no packages found matching boost-dev
    Running: sudo --askpass apt-get update
    Sorry, try again.
    Sorry, try again.
    sudo: 3 incorrect password attempts
    Traceback (most recent call last):
    ...
    
  4. Root, tty

    >>> sp.install("ssh")
    dpkg-query: no packages found matching ssh
    Running: sudo apt-get install -y --no-install-recommends ssh
    Reading package lists... Done
    ...
    
  5. Root, no-tty

    >>> sp._tool._sudo_str = "sudo --askpass "
    >>> sp.install("vlc")
    dpkg-query: no packages found matching vlc
    Running: sudo --askpass apt-get install -y --no-install-recommends vlc
    Reading package lists... Done
    Building dependency tree       
    
    

I think all casuistry is covered. 馃懆鈥嶐煆

@lasote

lasote approved these changes Oct 15, 2018

@uilianries
Copy link
Member

left a comment

LGTM

@jgsogo

This comment has been minimized.

Copy link
Member Author

commented Oct 15, 2018

Conflicts solved!

@lasote lasote added this to the 1.9 milestone Oct 15, 2018

@lasote lasote merged commit a8d81d8 into conan-io:develop Oct 15, 2018

2 checks passed

continuous-integration/jenkins/pr-head This commit looks good
Details
license/cla Contributor License Agreement is signed.
Details

@ghost ghost removed the stage: review label Oct 15, 2018

@jgsogo jgsogo deleted the jgsogo:issue/3285 branch Oct 15, 2018

@yangcha

This comment has been minimized.

Copy link
Contributor

commented Oct 31, 2018

sudo --askpass breaks Centos 6 package installation. The sudo version on Centos 6 is 1.8.6. The option of askpass for sudo version 1.8.7 or older is sudo -A.
So sudo -A will work on almost every platforms.

@yangcha

This comment has been minimized.

Copy link
Contributor

commented Nov 9, 2018

On docker lasote/conantests, given

from conans.client.tools.system_pm import SystemPackageTool

sp = SystemPackageTool()

Use cases:

  1. No-root, tty:
    >>> sp.install("boost-dev")
    dpkg-query: no packages found matching boost-dev
    Running: sudo apt-get update
    [sudo] password for tom:
    
  2. No-root, no-tty:
    >>> sp._tool._sudo_str = "sudo --askpass "
    >>> sp.install("boost-dev")
    dpkg-query: no packages found matching boost-dev
    Running: sudo --askpass apt-get install -y --no-install-recommends boost-dev
    sudo: no askpass program specified, try setting SUDO_ASKPASS
    Traceback (most recent call last):
    
  3. No-root, no-tty, with SUDO_ASKPASS
    With file pw.sh:
    #!/bin/bash
    echo 'invalid-root-pass'
    
    and running python with the env variable
    $ SUDO_ASKPASS=/home/conan/conan/pw.sh python
    
    >>> sp._tool._sudo_str = "sudo --askpass "
    >>> sp.install("boost-dev")
    dpkg-query: no packages found matching boost-dev
    Running: sudo --askpass apt-get update
    Sorry, try again.
    Sorry, try again.
    sudo: 3 incorrect password attempts
    Traceback (most recent call last):
    ...
    
  4. Root, tty
    >>> sp.install("ssh")
    dpkg-query: no packages found matching ssh
    Running: sudo apt-get install -y --no-install-recommends ssh
    Reading package lists... Done
    ...
    
  5. Root, no-tty
    >>> sp._tool._sudo_str = "sudo --askpass "
    >>> sp.install("vlc")
    dpkg-query: no packages found matching vlc
    Running: sudo --askpass apt-get install -y --no-install-recommends vlc
    Reading package lists... Done
    Building dependency tree       
    

I think all casuistry is covered.

Does this work for invoking command remotely via ssh?

grisumbras pushed a commit to grisumbras/conan that referenced this pull request Dec 27, 2018

Add --askpass argument to 'sudo' if it is not an interactive terminal (
鈥onan-io#3727)

* add param --askpass if not in interactive terminal

* typo

* add tests with --askpass in sudo command
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can鈥檛 perform that action at this time.