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

From: Jeff King <peff@peff.net>
Date: Feb 11, 2013, 17:27 UTC
Message-ID: <20130211172752.GH16402@sigill.intra.peff.net>
In-Reply-To: <7v38x2ojl1.fsf@alter.siamese.dyndns.org>

On Mon, Feb 11, 2013 at 09:18:18AM -0800, Junio C Hamano wrote:

Show 15 quoted lines

> That "shell-disabled" thing was to allow customizing the existing
> die() that triggers here:
> [...]
> 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() ...

OK, that is equivalent to what I said (or at least what I was trying to say :) ).

Show 13 quoted lines

> > 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).

Ah, I thought you were proposing shell-disabled _instead_ of Jonathan's patch, not in addition to.

Show 6 quoted lines

> 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.

Oh, absolutely. The easy case should be easy, and the hard case possible. But another way of doing that (which would also make life easier for admins who want to share config besides shell-disabled) would be:

  1. Give Jonathan's magic meaning to ~/git-shell-commands/help's exit
     code.

  2. Make /etc/git/shell-commands a fallback if ~/git-shell-commands
     does not exist.

That turns your /etc/git/shell-disabled into /etc/git/shell-commands/help. It is just as simple to do a site-wide change, still allows per-user overrides, and additionally gives people who _do_ want the interactive commands the ability to configure them site-wide instead of symlinking a directory into everybody's homedir.

The only downside is that it has the confusing "create this directory to turn on interactivity, then create a file in it to turn it back off" feature.

I admit I don't care too much, though. I have never actually used git-shell, as my systems are all either too small (i.e., users are trusted and have shell access) or too big (grown well beyond a single server that connects users straight to git-shell). In fact, there seems to be a lot of guessing in this thread about what people would want, as it seems none of us actually uses the feature. Maybe that is a sign it is being over-engineered. :)

-Peff