autocompletion is broken in restricted shell

autocompletion is broken in restricted shell

[Jan Kryl]

[-- Attachment #1: Type: text/plain, Size: 552 bytes --]

Hi,

after we updated zsh to 5.3 autocompletion stopped to work for us in restricted
shell. This is mainly due to "38692: IFS can't be changed in restricted mode”.
I suppose there is a good reason why setting IFS is not allowed in restricted mode
even though that it’s not apparent to me.

However at least I would like to fix another thing which breaks autocompletion and
that is using write redirections in autocompletion code. That can be avoided by using
2>&- instead of 2>/dev/null. A patch for review is attached.

thanks
-Jan


[-- Attachment #2: patch --]
[-- Type: application/octet-stream, Size: 536 bytes --]

--- a/Completion/compinit    Mon May  8 12:51:31 2017
+++ b/Completion/compinit    Mon May  8 12:51:40 2017
@@ -168,7 +168,7 @@
              _comp_caller_options=(${(kv)options[@]});
              setopt localoptions localtraps localpatterns ${_comp_options[@]};
              local IFS=$'\'\ \\t\\r\\n\\0\'';
-             builtin enable -p \| \~ \( \? \* \[ \< \^ \# 2>/dev/null;
+             builtin enable -p \| \~ \( \? \* \[ \< \^ \# 2>&-;
              exec </dev/null;
              trap - ZERR;
              local -a reply;

Re: autocompletion is broken in restricted shell

[Peter Stephenson]

On Mon, 8 May 2017 11:38:37 +0200
Jan Kryl <jan.kryl@nexenta.com> wrote:
> However at least I would like to fix another thing which breaks
> autocompletion and that is using write redirections in autocompletion
> code. That can be avoided by using 2>&- instead of 2>/dev/null. A
> patch for review is attached.

Hmmm... I'm not sure if this is deliberate, to minimise the chance of
errors.  But, as discussed elsewhere, we ignore write errors on
descriptors a lot of the time anyway, so I'm not sure it matters... and
I'm not sure if you're realistically going to get an error message from
there as long as the shell supports that syntax, so it's probably
reasonable to apply this and see what happens.

pws

Re: autocompletion is broken in restricted shell

[Bart Schaefer]

On Mon, May 8, 2017 at 9:48 AM, Peter Stephenson
<p.stephenson@samsung.com> wrote:
> On Mon, 8 May 2017 11:38:37 +0200
> Jan Kryl <jan.kryl@nexenta.com> wrote:
>> That can be avoided by using 2>&- instead of 2>/dev/null.
>
> I'm not sure if you're realistically going to get an error message from
> there as long as the shell supports that syntax, so it's probably
> reasonable to apply this and see what happens.

Although this won't matter 90+% of the time because "enable" rarely
emits an error, I think it's going to complain when there IS an error:

% () { print -u2 OOPS } 2>&-
zsh: write error

If you're going to get error output anyway, you might as well drop the
2> and get *useful* error output.

However, there are a zillion other places in completion where we
redirect stderr to /dev/null, so changing that single one in compinit
isn't going to fix the general problem.

Re: autocompletion is broken in restricted shell

[Bart Schaefer]

I know Peter already committed the patch included with this, but I think
it's worth revisiting:

On May 8, 11:38am, Jan Kryl wrote:
}
} after we updated zsh to 5.3 autocompletion stopped to work for us in
} restricted shell. This is mainly due to "38692: IFS can't be changed
} in restricted mode".
[...]
} I suppose there is a good reason why setting IFS is not allowed in
} restricted mode

If you look at workers/38692 there is a URL linking to a security exploit 
that is made possible by changing $IFS.

} However at least I would like to fix another thing which breaks
} autocompletion and that is using write redirections in autocompletion
} code. That can be avoided by using 2>&- instead of 2>/dev/null.

As I said in workers/41075 I don't think this is useful.  It also is
applicable only in restricted mode.  Looking again at workers/38692
I quote:

>> I don't think we ever expect the completion system to work properly
>> in restricted mode, do we? I would generally expect that any
>> environment involving functions other than extremely trivial ones
>> can't rely on restricted mode.

If we're already admitting that compsys is too complex to be reliable
in restricted mode, I think we should not have applied a patch that
only matters in restricted mode and that might cause spurious errors
in the far more common case.

Re: autocompletion is broken in restricted shell

[Peter Stephenson]

On Tue, 9 May 2017 22:02:47 -0700
Bart Schaefer <schaefer@brasslantern.com> wrote:
> If we're already admitting that compsys is too complex to be reliable
> in restricted mode, I think we should not have applied a patch that
> only matters in restricted mode and that might cause spurious errors
> in the far more common case.

I'm not that bothered, either way, because I can't judge either how much
could it's doing in restricted mode --- does it make the difference
between basic usability and not, or is it just another minor additional
problem --- or what problem it's going to cause in normal operation,
where you wouldn't expect to get an error with a recent version of the
shell.

I wonder if the answer at this stage is to remove the redirection
completely.

pws