Unable to autocomplete with sudo

nickvergessen commented 7 years ago

Hi there,

we from Nextcloud started using the completion and it works quite fine in general.

The only problem is, that it is impossible to run the stuff with e.g. sudo -u www-data. This is the recommanded easy-default for Nextcloud, to simply have all code and files owned by the www-data user. But I am unable to --generate-hook in this case.

I'm not sure if this is fixable, but would be very cool if so. I tried adding a copy of a generated hook script manually in /etc/bash_completion.d/ but when trying to modify the script to add sudo -u www-data on the calls, it seems to not execute properly anymore.

Any idea/workaround?

Cheers and keep it up

aik099 commented 7 years ago

Can you please write exact commands you're using and username you login with, that has hook installed for?

aik099 commented 7 years ago

For example:

I login using alex username
user alex has ~/.bashrc file with these line source <($HOME/web/code-insight/bin/code-insight _completion --generate-hook -p code-insight)
after that presuming, that code-insight binary is accessible in PATH I type code-insight TAB TAB and get auto-completion

stecman commented 7 years ago

I've just tested that bash-completion for sudo [args...] mysymfonycommand correctly hands off completion to the handler for mysymfonycommand (at least on Debian stretch). @nickvergessen - are you running these sudo commands from a non-privileged user, or under the root account? Kind of an odd question, but I've experienced problems with bash-completion stopping working under sudo su etc. Breakdown of steps per @aik099's comment would be great

nickvergessen commented 7 years ago

So in my case it is as follows:

local user: nickvergessen script: /var/www/nextcloud/occ

occ (calling console.php) has the following section:

    $user = posix_getpwuid(posix_getuid());
    $configUser = posix_getpwuid(fileowner(OC::$configDir . 'config.php'));
    if ($user['name'] !== $configUser['name']) {
        echo "Console has to be executed with the user that owns the file config/config.php" . PHP_EOL;
        exit(1);
    }

owner of config.php has to be the apache user www-data. So instead of /var/www/nextcloud/occ I need to run: sudo -u www-data /var/www/nextcloud/occ

I guess the only way is to "ignore" the user check for the _complete command? Because it is not possible to add the sudo to the call of your generated script:

RESULT="$(/var/www/nextcloud/occ _completion </dev/null)";

nickvergessen commented 7 years ago

Since ignoring the user check does not work, because we can not get a connection to the database because we can not read the config,... I tried to continue with the other route.

With:

RESULT="$(sudo -u www-data /var/www/nextcloud/occ _completion </dev/null)";

I receive:

  [RuntimeException]                                                               
  Failed to configure from environment; Environment var CMDLINE_CONTENTS not set.  

_completion [-g|--generate-hook] [-p|--program PROGRAM] [-m|--multiple] [--shell-type [SHELL-TYPE]]

I guess this is, because www-data has nologin

aik099 commented 7 years ago

I haven't tested, but does sudo ls -la:

show only folders that root access
just folders that current user invoking sudo command can access

?

If this is the 2nd case, then I don't see possible workaround for sudo. Also the problem with sudo is that it will ask current user's password. We can just ignore that fact and try to run sudoed command somehow.

nickvergessen commented 7 years ago

$ sudo ls -la config/
drwxrwxr-x  2 www-data www-data  4096 Nov 21 16:10 .
drwxr-xr-x 23 nickv    nickv     4096 Nov 21 16:10 ..
-rw-rw----  1 www-data www-data  2176 Nov 21 16:10 config.php

$ tail config/config.php 
tail: cannot open 'config/config.php' for reading: Permission denied

So it lists the config file which I cannot read/open.

aik099 commented 7 years ago

It will work if you have entered password previously or you're allowed to run sudo without entering a password at all. Maybe then it's possible to tweak auto-complete as well.

nickvergessen commented 7 years ago

Yeah, so any idea how to fix this? entering sudo password before doesnt seem to help

aik099 commented 7 years ago

Not really.

You can create basic bash autocomplete hook using compgen and complete commands and see if it works through sudo.

If it does, then it means all ENV vars are properly proxied to completion script and it's stecman/symfony-console-completion that doesn't handle it.
If it doesn't, then bash doesn't support auto-complete over sudo and stecman/symfony-console-completion can't help, because it's working based on ENV vars provided by bash.

nickvergessen commented 7 years ago

I have:

_nhs() 
{
    local cur
    # Pointer to current completion word.
    # By convention, it's named "cur" but this isn't strictly necessary.

    COMPREPLY=()   # Array variable storing the possible completions.
    cur=${COMP_WORDS[COMP_CWORD]}

    case "$COMP_CWORD" in
        1)
        SERVERS='9 10 11 12'
        COMPREPLY=( $( compgen -W "$SERVERS" -- $cur ) );;
    esac

    return 0
}
complete -F _nhs nhs

and can do:

sudo nhs <tab>
10 11 12 9

The problem is the RESULT call in your script that needs sudo as well

RESULT="$(/home/nickv/Nextcloud/12/server/occ _completion </dev/null)";

So I replaced https://github.com/nextcloud/3rdparty/blob/b6ef8a42ae8c7d4d69c6acdf919072a7989648d7/stecman/symfony-console-completion/src/HookFactory.php#L44-L44 with

RESULT="$(sudo -u www-data %%completion_command%% </dev/null)";

but then sudo -u www-data ./occ <tab> brings:

  [RuntimeException]                                                               
  Failed to configure from environment; Environment var CMDLINE_CONTENTS not set.  

_completion [-g|--generate-hook] [-p|--program PROGRAM] [-m|--multiple] [--shell-type [SHELL-TYPE]]

Not fixabled?

aik099 commented 7 years ago

I know that /usr/bin/env can pass through env vars. I'm not sure if it can pass env vars from non-sudo to sudo calls.

Does RESULT="$(sudo -u www-data /usr/bin/env %%completion_command%% </dev/null)"; work?

stecman commented 7 years ago

@nickvergessen can you provide any more detail about your environment? I've just tested this on my machine and completion works fine calling with sudo occ [tab] and sudo -u www-data occ [tab] from what I can see:

$ uname -a
Linux 3.13.0-37-generic #64-Ubuntu SMP Mon Sep 22 21:28:38 UTC 2014 x86_64 x86_64 x86_64 GNU/Linux

$ echo $BASH_VERSION
4.3.46(1)-release

$ php -v
PHP 5.6.29-1+deb.sury.org~xenial+1 (cli) 
Copyright (c) 1997-2016 The PHP Group
Zend Engine v2.6.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.6-dev, Copyright (c) 1999-2016, by Zend Technologies

nickvergessen commented 7 years ago

Does RESULT="$(sudo -u www-data /usr/bin/env %%completion_command%% </dev/null)"; work?

Same issue:

$ sudo -u www-data ./occ _completion -g --shell-type bash

function _occ_feacda5c68cc086b_complete {

    local CMDLINE_CONTENTS="$COMP_LINE"
    local CMDLINE_CURSOR_INDEX="$COMP_POINT"
    local CMDLINE_WORDBREAKS="$COMP_WORDBREAKS";

    export CMDLINE_CONTENTS CMDLINE_CURSOR_INDEX CMDLINE_WORDBREAKS

    local RESULT STATUS;

    RESULT="$(sudo -u www-data /usr/bin/env ./occ _completion </dev/null)";

    STATUS=$?;

    local cur mail_check_backup;

    mail_check_backup=$MAILCHECK;
    MAILCHECK=-1;

    _get_comp_words_by_ref -n : cur;

    if [ $STATUS -eq 200 ]; then
        _filedir;
        return 0;

    elif [ $STATUS -ne 0 ]; then
        echo -e "$RESULT";
        return $?;
    fi;

    COMPREPLY=(`compgen -W "$RESULT" -- $cur`);

    __ltrim_colon_completions "$cur";

    MAILCHECK=mail_check_backup;
};

if [ "$(type -t _get_comp_words_by_ref)" == "function" ]; then
    complete -F _occ_feacda5c68cc086b_complete "./occ";
else
    >&2 echo "Completion was not registered for ./occ:";
    >&2 echo "The 'bash-completion' package is required but doesn't appear to be installed.";
fi

$ eval $(sudo -u www-data ./occ _completion -g --shell-type bash)

$ sudo -u www-data ./occ 

  [RuntimeException]                                                               
  Failed to configure from environment; Environment var CMDLINE_CONTENTS not set.  

_completion [-g|--generate-hook] [-p|--program PROGRAM] [-m|--multiple] [--shell-type [SHELL-TYPE]]

System info:

$ uname -a
Linux nickv-infinity 4.4.0-53-generic #74-Ubuntu SMP Fri Dec 2 15:59:10 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux
$ echo $BASH_VERSION
4.3.42(1)-release
$ php -v
PHP 7.0.14-2+deb.sury.org~xenial+1 (cli) ( NTS )
Copyright (c) 1997-2016 The PHP Group
Zend Engine v3.0.0, Copyright (c) 1998-2016 Zend Technologies
    with Zend OPcache v7.0.14-2+deb.sury.org~xenial+1, Copyright (c) 1999-2016, by Zend Technologies
    with Xdebug v2.5.0, Copyright (c) 2002-2016, by Derick Rethans

I guess the most important thing is the changed owner ship. I made config.php owned by www-data:www-data, but I guess you have that as well. Not sure what could be of interest other then that.

$ ls -la config/
insgesamt 136
drwxrwxr-x  2 nickv    www-data  4096 Dez 19 15:47 .
drwxr-xr-x 22 nickv    nickv     4096 Dez 19 15:47 ..
-rw-rw----  1 www-data www-data  2059 Dez 19 15:47 config.php
-rw-rw-r--  1 nickv    www-data 41687 Dez 15 16:36 config.sample.php
-rw-rw-r--  1 nickv    www-data   225 Dez 13 17:59 .htaccess

nickvergessen commented 7 years ago

Seeing you ran your eval $(... without sudo -u www-data, your setup must be different because for me that doesn't even work:

$ /home/nickv/Nextcloud/12/server/occ
Cannot write into "config" directory!
This can usually be fixed by giving the webserver write access to the config directory

See https://docs.nextcloud.com/server/11/go.php?to=admin-dir_permissions

nickvergessen commented 7 years ago

Okay, I got it working using the following instead of https://github.com/nextcloud/3rdparty/blob/b6ef8a42ae8c7d4d69c6acdf919072a7989648d7/stecman/symfony-console-completion/src/HookFactory.php#L44-L44:

RESULT="$(sudo -u www-data /usr/bin/env CMDLINE_CONTENTS='$COMP_LINE' CMDLINE_CURSOR_INDEX='$COMP_POINT' CMDLINE_WORDBREAKS='$COMP_WORDBREAKS' %%completion_command%% </dev/null)";

Would be nice if this could make it into your repo in a clean way 😃

aik099 commented 7 years ago

I guess we can specify --sudo-username option (new) during hook generation and if set it will:

use sudo command for that user
transfer all env (better not to hardcode env variables) to sudo

nickvergessen commented 7 years ago

I just noticed, that now only commands can be autocompleted:

$ sudo -u www-data /home/nickv/Nextcloud/12/server/occ _completion 
app:check-code                      encryption:disable                  maintenance:mode
app:disable                         encryption:enable                   maintenance:repair
app:enable                          encryption:encrypt-all              maintenance:singleuser
....

But I guess that is because I only forwarded some of the enviroment variables?

aik099 commented 7 years ago

You can do sudo -E (according to http://unix.stackexchange.com/questions/202292/pass-environment-variables-values-to-a-program) to pass all env to sudoed script (I wish I could have found this sooner). This way following code should work as well:

RESULT="$(sudo -E -u www-data %%completion_command%% </dev/null)";

nickvergessen commented 7 years ago

👍 that one works

aik099 commented 7 years ago

I guess you can send PR, that will add --sudo-user option with required value and then when specified during hook generation the sudo version of RESULT variable would be used for both BASH and ZSH.

nickvergessen commented 7 years ago

https://github.com/stecman/symfony-console-completion/issues/73

stecman commented 7 years ago

$ /home/nickv/Nextcloud/12/server/occ
Cannot write into "config" directory!
This can usually be fixed by giving the webserver write access to the config directory

See https://docs.nextcloud.com/server/11/go.php?to=admin-dir_permissions

@nickvergessen to be honest this seems like a permissions / design issue with Nextcloud, not the completion system. A few suggestions for working around this:

Configure file/dir permissions so that www-data and the owning user can both write to those directories:
```
$ chown -R application-code-owner:www-data config/ ...
$ chmod 775 config/
$ chmod 660 config/*

# or with sticky groups on directories if your program creates nested
# directories that need the same permissions
chmod 2775 config/
```
If you have all of the code owned by an application user like nextcloud, this also potentially solves this from the end of that help page:

These strong permissions prevent upgrading your Nextcloud server; see Setting Permissions for Updating for a script to quickly change permissions to allow upgrading.

Since it means you can do something like:
```
sudo su application-user
cd /var/www/nextcloud
# do upgrading things with all the write permissions needed
```
Alternatively you could remove the check for write permissions during completion, unless write access is actually needed by the completion in your application.

nickvergessen commented 7 years ago

Well most enviroments don't have an application-user, also all docs and tutorials out there mention www-data to be used, but for that one sudo su www-data is not allowed, because it is a no-login account.

The patch is already done (see #73 ) and we will be easily able to use this, if you don't want to merge it, we will just add the _completion -g command a second time with the fixed version, works for me, but I think it would be cooler if it is all in one place.

stecman commented 7 years ago

Sorry, I was meaning [application-user] as a placeholder for a user that's been added to run Nextcloud (or just the user's account that installed it) - eg.

$ sudo useradd --system nextcloud
$ sudo su nextcloud
# or
$ sudo -u nextcloud

I appreciate that you've made a PR for this, but I think it adds unnecessary complexity for something that's very specific to the way Nextcloud currently works, and can be avoided with a small change to filesystem permissions. If you couldn't fix this any other clean way I wouldn't have a problem with it, but it looks like there are quite a few good solutions that don't require changes to this library.

In addition to the two suggestions above, another relatively clean solution might be extending CompletionCommand and HookFactory in your code to get the functionality you're after. I'm happy to accept a PR that makes it easier to do this, like getting HookFactory from a method in CompletionCommand instead of instantiating it inline.

nickvergessen commented 7 years ago

We think that adding a user is too complicated, because sometimes permissions might change and all the docs would need adjustment and so on.

So if you don't want to merge this because it adds too much parameters ( I get and see that as well), I will try to extend your classes and see how that works. As far as I can see you use protected everywhere (no private) so that should work.

nickvergessen commented 7 years ago

PS: okay the HookFactory can not be extended due to final If you could get rid of that in a future version it would allow things to be easier, but all the template needs to be overwritten anyway, so just the helper functions would be "saved".

stecman commented 7 years ago

Hmm, so it is.. I think I did that with the intention that HookFactory would essentially be a template storage thing and no more. You're welcome to rejigger HookFactory to make it easier to inject or override specific parts of the replacements, and remove the final keyword, though that's straying into depending on implementation details which isn't ideal for you either :/

Is my second suggestion feasible? Seems like completion shouldn't need write access to run:

...remove the check for write permissions during completion, unless write access is actually needed by the completion in your application.

ghost commented 5 years ago

@nickvergessen: were you able to find a reliable procedure to enable autocompletion yet? i am struggeling around ...

nickvergessen commented 5 years ago

Not really

RonaldBarnes commented 1 year ago

we from Nextcloud started using the completion and it works quite fine in general.

The only problem is, that it is impossible to run the stuff with e.g. sudo -u www-data. This is the recommanded easy-default for Nextcloud, to simply have all code and files owned by the www-data user. But I am unable to --generate-hook in this case.

I'm not sure if this is fixable, but would be very cool if so. I tried adding a copy of a generated hook script manually in /etc/bash_completion.d/ but when trying to modify the script to add sudo -u www-data on the calls, it seems to not execute properly anymore.

Any idea/workaround?

Have a look at a pull request for full bash tab completion via aliasing occ to sudo --user ... php .../occ as well as a complete.occ script to handle the tab completion on occ.

https://github.com/nextcloud/server/pull/35451

Will create valid alias, run it, add it to ~/.bash_aliases, add it to the SUDO_USER's ~/.bash_aliases if appropriate, run the completion script, offer to copy it to /etc/bash_completion.d/, check that the web server user it found is also the owner of occ, etc.

stecman commented 1 year ago

@RonaldBarnes thanks, good to see how you're handling it there. Does completion actually need to run as that user though?

I believe the underlying issue in this ticket is/was that permissions checks were running during the completion command, which seems like something that could be skipped intelligently. However, if you need to run as a different user to read config files that have locked down permissions, this would make sense.

RonaldBarnes commented 1 year ago

@RonaldBarnes thanks, good to see how you're handling it there. Does completion actually need to run as that user though?

Thanks!

Yes, occ (the php script) needs to be run by its owner, even user root doesn't qualify:

# php ./occ
Console has to be executed with the user that owns the file config/config.php
Current user id: 0

I believe the underlying issue in this ticket is/was that permissions checks were running during the completion command, which seems like something that could be skipped intelligently. However, if you need to run as a different user to read config files that have locked down permissions, this would make sense.

The permission checks seem like a good idea that shouldn't be skipped -- let the OS enforce access restrictions in another layer of security.

But the alias occ="sudo --user=... php .../occ" method complies with the file access and user escalation permission checks nicely.

nickvergessen commented 1 year ago

Thanks a lot for all the work @RonaldBarnes Happy to see this finally being picked up, unluckily I'm out of time at the moment and can therefore not assist you with the issue/pull request. But seems others are helping :)

MichaIng commented 1 year ago

I got it working like this:

$ . <(sudo -u www-data --preserve-env=SHELL php /var/www/nextcloud/occ _completion -g | sed 's|/var/www/nextcloud/occ|sudo -u www-data --preserve-env=CMDLINE_CONTENTS,CMDLINE_CURSOR_INDEX,CMDLINE_WORDBREAKS php /var/www/nextcloud/occ|')
$ alias occ='sudo -u www-data php /var/www/nextcloud/occ'
$ occ
Display all 124 possibilities? (y or n)
activity:send-mails                    config:app:get                         dav:sync-system-addressbook            encryption:status                      integrity:sign-core                    notification:test-push                 tag:edit                               user:delete
app:disable                            config:app:set                         db:add-missing-columns                 files:cleanup                          l10n:createjs                          notify_push:log                        tag:list                               user:disable
app:enable                             config:import                          db:add-missing-indices                 files:repair-tree                      list                                   notify_push:metrics                    theming:config                         user:enable
app:getpath                            config:list                            db:add-missing-primary-keys            files:scan                             log:file                               notify_push:reset                      trashbin:cleanup                       user:info
app:install                            config:system:delete                   db:convert-filecache-bigint            files:scan-app-data                    log:manage                             notify_push:self-test                  trashbin:expire                        user:lastseen
app:list                               config:system:get                      db:convert-mysql-charset               files:transfer-ownership               log:tail                               notify_push:setup                      trashbin:restore                       user:list
app:remove                             config:system:set                      db:convert-type                        group:add                              log:watch                              preview:repair                         trashbin:size                          user:report
app:update                             dav:create-addressbook                 encryption:change-key-storage-root     group:adduser                          maintenance:data-fingerprint           preview:reset-rendered-texts           twofactorauth:cleanup                  user:resetpassword
background-job:execute                 dav:create-calendar                    encryption:decrypt-all                 group:delete                           maintenance:mimetype:update-db         ransomware_protection:block            twofactorauth:disable                  user:setting
background-job:list                    dav:delete-calendar                    encryption:disable                     group:info                             maintenance:mimetype:update-js         security:bruteforce:reset              twofactorauth:enable                   versions:cleanup
background:ajax                        dav:list-calendars                     encryption:enable                      group:list                             maintenance:mode                       security:certificates                  twofactorauth:enforce                  versions:expire
background:cron                        dav:move-calendar                      encryption:encrypt-all                 group:removeuser                       maintenance:repair                     security:certificates:import           twofactorauth:state                    workflows:list
background:webcron                     dav:remove-invalid-shares              encryption:list-modules                help                                   maintenance:repair-share-owner         security:certificates:remove           update:check
broadcast:test                         dav:retention:clean-up                 encryption:migrate-key-storage-format  integrity:check-app                    maintenance:theme:update               status                                 upgrade
check                                  dav:send-event-reminders               encryption:set-default-module          integrity:check-core                   maintenance:update:htaccess            tag:add                                user:add
config:app:delete                      dav:sync-birthday-calendar             encryption:show-key-storage-root       integrity:sign-app                     notification:generate                  tag:delete                             user:add-app-password

Only the SHELL variable is needed to generate the hook script. Alternatively --shell-type bash or --shell-type zsh can be used, in which cases the SHELL variable is not required.
Only the three completion-related variables, exported within the hook script, need to be passed through sudo.

What indeed would help is:

--sudo-user=USER and/or reading it from SUDO_USER variable, to add sudo -u USER --preserve-env=CMDLINE_CONTENTS,CMDLINE_CURSOR_INDEX,CMDLINE_WORDBREAKS to the hook script automatically.
However, the php command is required as well in case of Nextcloud, since the occ script is not executable OOTB. More logic could solve this: [[ -x $completionCommand ]] || completionCommand="php $completionCommand" (respectively doing this in PHP hook factory already.

stecman / symfony-console-completion

Unable to autocomplete with sudo #71