Re: [RFC/PATCH] shell: allow 'help' command to disable interactive shell

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 11, 2013, 17:18 UTC
Message-ID: <7v38x2ojl1.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20130211160057.GA16402@sigill.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 25 quoted lines

> On Sun, Feb 10, 2013 at 11:17:24PM -0800, Junio C Hamano wrote:
>
>> Jonathan Nieder <jrnieder@gmail.com> writes:
>> 
>> > Isn't that a criticism of the git-shell-commands facility in general?
>> > If it is common to have a lot of users with distinct home directories
>> > but all with git-shell as their login shell, then the
>> > git-shell-commands should not go in their home directory to begin
>> > with, no?
>> 
>> You can give one set of commands to some users while restricting
>> others, no?
>
> But that seems to me to argue against /etc/git/shell-disabled or
> similar, which would apply to every user. Or are you proposing that the
> check be:
>
>   if -d ~/git-shell-commands; then
>           : ok, interactive
>   elif -x /etc/git/shell-disabled; then
>           exec /etc/git/shell-disabled
>   else
>           echo >&2 'go away'
>           exit 1
>   fi

That "shell-disabled" thing was to allow customizing the existing die() that triggers here:

	} else if (argc == 1) {
		/* Allow the user to run an interactive shell */
		cd_to_homedir();
		if (access(COMMAND_DIR, R_OK | X_OK) == -1) {
			die("Interactive git shell is not enabled.\n"
			    "hint: ~/" COMMAND_DIR " should exist "
			    "and have read and execute access.");
		}
		run_shell();
		exit(0);

so it is more like

	if ! test -d $HOME/git-shell-commands
	then
		if test -x /etc/git/shell-disabled
                then
			exec /etc/git/shell-disabled
		else
			die Interactive is not enabled
		fi
	fi
        ... do whatever in run_shell() ...

> That at least means you can apply _whether_ to disable the shell
> selectively for each user (by providing or not a git-shell-commands
> directory), but you cannot individually select the script that runs for
> that user.  But it's probably still flexible enough;...

Such a flexibility is not a goal of /etc/git/shell-disabled. The sole goal is to make the life easier for those site owners that do not want any interactive shell access to give more friendly and customized error message.

Those who want further flexibility can exit with non-zero from the "help" (which is still a misnomer for a hook to disable interactive for the user).

My primary objection is that implementing only that "more flexible but requires more configuration work" solution without giving simpler solution (i.e. just one thing to configure) to the majory of site owners who only have simpler problem to solve (i.e. just want to customize "no interactive here"), and saying that the latter can be done on top. It is backwards mentality.