Deactivated security check at compinit #396
Conversation
|
See #397 for explanations. |
base/core/load.zsh
Outdated
| @@ -31,7 +31,12 @@ __zplug::core::load::from_cache() | |||
|
|
|||
| # Plugins with defer-level set | |||
| source "$_zplug_cache[defer_1_plugin]" | |||
| compinit -d "$ZPLUG_HOME/zcompdump" | |||
| if [ ${UID} ] && [ -n ${SUDO_USER} ] ; then | |||
There was a problem hiding this comment.
[[ is better, and { is unneeded
- if [ ${UID} ] && [ -n ${SUDO_USER} ] ; then
+ if [[ -n $UID ] && [[ -n $SUDO_USER ]] ; then
There was a problem hiding this comment.
one [ to much in front of SUDO_USER. Variables in {} is in this case unneeded but general good style, this way variables can be chained and/or substituted.
|
Well, there is |
|
I had put WIP in the title because in the initial PR, I had just completely disabled the security. I guess it doesn't make sense anymore since we seem to have a proper fix. However, I'm a bit confused by the compinit line in core.zsh, which alwas has the security disabled. Is this because it's supposed to load a dump file already generated by load.zsh ? |
toadjaune
changed the title
|
After re-reading the change request, I'm not sure what you guys are asking for : |
|
I would say this: This will not work because syntax is wrong: And both check a condition that doesn't matter: |
|
Changed according to your latest suggestion, @NemesisRE. Everybody happy with this ? |
|
LGTM |
|
I'll check it later |
|
@b4b4r07, any news ? |
Squashed commit of the following Pull Request: zplug#396 commit 8d08dc1 Author: Arnaud Venturi <[email protected]> Date: Mon Jul 24 18:06:27 2017 +1000 Update load.zsh commit 05130e1 Author: Arnaud Venturi <[email protected]> Date: Sat Jun 3 22:34:17 2017 +1000 Included the style change requested in PR commit 75fbff9 Author: Arnaud Venturi <[email protected]> Date: Thu Apr 27 16:26:00 2017 +1000 Allowed insecure compinit when using sudo commit 467075d Author: Arnaud Venturi <[email protected]> Date: Fri Apr 21 12:52:18 2017 +1000 Deactivated security check at compinit
|
How about a more generic COMPINIT_ARGS or otherwise open this up to more use cases? I'd like to use this to solve the issue of multiple users + home-brew #428 |
|
@g0t4 I'm not sure about that. I feel however like the simple use of sudo is way more common, and should work out-of-the box, both in terms of usability and security (we're not talking about deleting the security check, but only remove the second execution) Don't get me wrong, I agree that we need a manual override, I would just prefer to treat it as a separate issue. |
|
This is still an annoying issue while using sudo. How about a specific variable (maybe ZPLUG_COMPINIT_UNSECURE or something else) well documented and disabled by default. Something like this:
What do you think? |
|
any news on this issue? The environment variable (either ZPLUG_COMPINIT_UNSECURE, ZSH_DISABLE_COMPFIX or COMPINIT_ARGS) diabled by default seems reasonable? what to you think? |
|
This is still something I always have to patch myself when using my zsh with zplug. :/ |
No description provided.