From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 22219 invoked by alias); 2 Sep 2016 05:24:17 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 39156 Received: (qmail 13241 invoked from network); 2 Sep 2016 05:24:17 -0000 X-Qmail-Scanner-Diagnostics: from out4-smtp.messagingengine.com by f.primenet.com.au (envelope-from , uid 7791) with qmail-scanner-2.11 (clamdscan: 0.99.2/21882. spamassassin: 3.4.1. Clear:RC:0(66.111.4.28):SA:0(0.0/5.0):. Processed in 0.301343 secs); 02 Sep 2016 05:24:17 -0000 X-Spam-Checker-Version: SpamAssassin 3.4.1 (2015-04-28) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=0.0 required=5.0 tests=T_DKIM_INVALID autolearn=unavailable autolearn_force=no version=3.4.1 X-Envelope-From: d.s@daniel.shahaf.name X-Qmail-Scanner-Mime-Attachments: | X-Qmail-Scanner-Zip-Files: | Received-SPF: none (ns1.primenet.com.au: domain at daniel.shahaf.name does not designate permitted sender hosts) DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= daniel.shahaf.name; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-sasl-enc:x-sasl-enc; s=mesmtp; bh=F/2HepAR7y8khcM8 Wkae7YuDf6g=; b=TYhMF02FVH9wz1K+r7hEzxhpVa3Xr7aHGZdP9qUbKUhTq9rS oJGDpd+UlcAqLY2q0xUqsmncllNAORRJhapuyyfKUhdPLPEFqSqqO1oI/79mFyA4 Fsi/26//d+BTeXHn84JWnMJ1uKxvxV7QVPjASIlERFVmDuPKWEB4/kEgoik= DKIM-Signature: v=1; a=rsa-sha1; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-sasl-enc:x-sasl-enc; s=smtpout; bh=F/2HepAR7y8khcM 8Wkae7YuDf6g=; b=Twje3nD34qmoN8FQIV/DalIbIs7gePk1nXbjHni5VkVENpy 2bWispcWHT9LfTLNMb9Zr6wozgELngiswn1kYGov4BA0za4aMvnq7JN7VQ5R2BmZ kHkZ8Gy3+EFA60xA/y59XnvLeIW+pnh64OmP/OpOGH/HvBn0lO+MTu8rDT0k= X-Sasl-enc: Yfs1gOcZSj3H0rlwt/UvWZiqlqlJ8yDW2ojwA3XMEm3m 1472793851 Date: Fri, 2 Sep 2016 05:23:42 +0000 From: Daniel Shahaf To: Oliver Kiddle Cc: zsh-workers@zsh.org Subject: Re: [PATCH] _virsh (Was: Re: zsh virsh completion) Message-ID: <20160902052342.GA8514@fujitsu.shahaf.local2> References: <8eb6dce0-50d7-5ab2-503a-194c1de2e45d@redhat.com> <20160713045957.GA3893@tarsus.local2> <9968da53-c1fd-fa2a-f30c-c74f884d2478@redhat.com> <20160720065832.GA28939@tarsus.local2> <699166a0-b0f0-452c-2561-b7e3cc952062@redhat.com> <25001.1469117569@hydra.kiddle.eu> <20160722071927.GG2521@tarsus.local2> <12554.1472678120@hydra.kiddle.eu> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <12554.1472678120@hydra.kiddle.eu> User-Agent: Mutt/1.5.23 (2014-03-12) Oliver Kiddle wrote on Wed, Aug 31, 2016 at 23:15:20 +0200: > On 22 Jul, Daniel Shahaf wrote: > > Secondly, you don't touch on what we would do when the 'gain-root' style > > is unset. Given Marko's later email that virt-admin is not usable by > > non-root users, perhaps we should do this: > > I'd suggest that it doesn't do anything special. Just run virsh > list. Testing EUID might be correct but you can't actually be sure > it won't work, perhaps due to groups or some RBAC mechanism. If it > doesn't you'd end up with a message like: > No matches for: `domains' > In some cases, _wanted -x should perhaps be used. > > Putting the logic in _call_program also has an impact on this > question because while virt-admin might be unusable to non-root > users, some other commands might be partially usable. sudo may > just allow it to get more matches. All this makes sense. However, your patch seems to implement something slightly different: the patch executes sudo when (-p was passed, and) 'gain-privs' is either unset or set to true, whereas the above paragraphs seem to be arguing for *not* using sudo when the style is unset? Perhaps I misunderstood your argument. I have no strong opinion regarding what gain-privs should default to when unspecified. > How about something like the following? _sudo sets the _comp_priv_prefix > variable to provide a prefix to match those for the current > command-line. If _call_program is called with -p, it will default > to using this prefix unless overridden, either by a gain-privs style > or the command style with a - prefix. > This approach also makes sense to me. It does, however, require us (completion file authors) to be *very* careful / disciplined with uses of -p to avoid possibly executing parts of the command line as root. Executing a command as sudo upon pressing could have undesired results in two ways: if the user _has_ sudo, executing the command with sudo could surprise her; and if user _doesn't_ have sudo, executing the command might result in an email from sudo to the root user. This change might deserve a NEWS entry? > This doesn't include an (( EUID )) check because it might be applicable > where permissions other than root are gained. I see: the privileges gained need not be privileges on the local system. > > It'd perhaps be useful if > the -p option to _call_program also caused it to add something to > $curcontext when looking up the command style but I'm not sure where > that could be done as we already have the tag and argument fields > filled. Any ideas? I suppose it fits best in the "command" part of the context, e.g., . _f() { : $(_call_program lipsum echo lorem ipsum) } sudo f . could look up :completion::complete:f-under-sudo::lipsum or so. (Replace "f-under-sudo" by any string that identifies both 'f' and 'sudo' unambiguously.) I don't think there's a compatibility problem here: if we add -p to an existing _call_program invocation we should change the tag that invocation uses, so might at the same time change the context the tag is looked up in. > Besides -u/--user, are other sudo options needed? > All of these seem potentially needed: -g -P (whom to execute as) -r -t (on my system they're about SELinux; not sure about their meaning on other platforms) -h (where to execute) -H -E -i (execution environment) That's based on the sudo manpage in Debian stable, sudo 1.8.10. Come to think of it, sudoers(5) allows per-command defaults, so if the completion of the command 'foo' invokes «_call_program -b bar», the sudoers(5) Defaults!foo clauses wouldn't fire. Hopefully that will do nothing worse than failing to find completion. > Oliver > > diff --git a/Completion/Base/Core/_main_complete b/Completion/Base/Core/_main_complete > index 9c4cfac..c292ce7 100644 > --- a/Completion/Base/Core/_main_complete > +++ b/Completion/Base/Core/_main_complete > @@ -38,6 +38,7 @@ local func funcs ret=1 tmp _compskip format nm call match min max i num\ > _saved_colors="$ZLS_COLORS" \ > _saved_colors_set=${+ZLS_COLORS} \ > _ambiguous_color='' > +local -a _comp_priv_prefix Could we please not introduce any more short opaque names? "priv" is ambiguous/unclear. Perhaps _comp_gain_privs_prefix. (Or perhaps the style name should be "gain-privileges".) > diff --git a/Completion/Unix/Command/_sudo b/Completion/Unix/Command/_sudo > index 63ac37f..1ceefa2 100644 > --- a/Completion/Unix/Command/_sudo > +++ b/Completion/Unix/Command/_sudo > @@ -48,7 +48,7 @@ else > '(-H --set-home -i --login -s --shell -e --edit)'{-H,--set-home}"[set HOME variable to target user's home dir]" \ > '(-P --preserve-groups -i -login -s --shell -e --edit)'{-P,--preserve-groups}"[preserve group vector instead of setting to target's]" \ > '(-)1:command: _command_names -e' > - '*::arguments: _normal' > + '*::arguments:{ _comp_priv_prefix=( $words[1] -n ${(kv)opt_args[(I)(-u|--user)]} ) ; _normal }' You removed the space that followed the last colon; wouldn't that cause _arguments to try and exec the anonymous block with arguments? I.e., "*::arguments:foo" executes «foo $expl», so wouldn't the above line cause «{ ... ; _normal } $expl» to be executed? Should $_comp_priv_prefix be left unset when (( ${+funcstack[(r)_ssh]} ))? Thanks for taking care of this. Cheers, Daniel > ) > fi > >