Re: [PATCH 2/2] shell: pay attention to exit status from 'help' command

> How is this for an alternative? Have shell.c look for a
>        [shell]
>                missing_commands_directory = "Stuff is broke."
> setting. If the setting is missing, then it prints the default message
> (the current message). That way, there's a default setting, there can
> be a system-wide message, there can be a user specific message, and
> those messages can be set via `git-config`.

> If I disable git-shell's interactive mode by removing the
> ~/git-shell-commands directory, then attempts to use 'ssh' with the
> git account interactively produce an error message intended for the
> administrator:
>
>         $ ssh git@myserver
>         fatal: Interactive git shell is not enabled.
>         hint: ~/git-shell-commands should exist and have read and execute access.
>         $
>
> That is helpful for the new admin who is wondering "What? Why isn't
> the git-shell I just set up working?", but once the site setup is
> finished, it is better to give the user a friendly hint that she is on
> the right track, like GitHub does:
>
>         Hi <username>! You've successfully authenticated, but
>         GitHub does not provide shell access.
>
> An appropriate greeting might even include more complex information,
> like a list of repositories the user has access to.  If the
> git-shell-commands directory exists and contains a "help" script, we
> already run it when the shell is run without any commands, giving the
> server a chance to provide a custom message.  Unfortunately, the
> presence of the git-shell-commands directory means we also enter an
> interactive mode, prompting and accepting commands (of which there may
> be none) from the user, which many servers would not want.  To solve
> this, we abort the interactive shell on a non-zero exit code from the
> "help" script.  This lets the server say whatever it likes, and then
> hang up.
>
> Downside: this will prevent interactive git-shell logins in existing
> setups where the "help" script exits with nonzero status by mistake.
> Hopefully those are rare enough to not cause much trouble in practice.
>
> Reported-by: Ethan Reesor <firelizzard@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> Improved-by: Jeff King <peff@peff.net>
> ---
>  Documentation/git-shell.txt | 20 ++++++++++++++++++++
>  shell.c                     | 10 ++++++++--
>  2 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-shell.txt b/Documentation/git-shell.txt
> index 4fe93203..60051e63 100644
> --- a/Documentation/git-shell.txt
> +++ b/Documentation/git-shell.txt
> @@ -59,6 +59,26 @@ users to list repositories they have access to, create, delete, or
>  rename repositories, or change repository descriptions and
>  permissions.
>
> +If the `help` command exists and exits with nonzero status, the
> +interactive shell is aborted.
> +
> +EXAMPLE
> +-------
> +
> +To disable interactive logins, displaying a greeting instead:
> ++
> +----------------
> +$ chsh -s /usr/bin/git-shell
> +$ mkdir $HOME/git-shell-commands
> +$ cat >$HOME/git-shell-commands/help <<\EOF
> +#!/bin/sh
> +printf '%s\n' "Hi $USER! You've successfully authenticated, but I do not"
> +printf '%s\n' "provide interactive shell access."
> +exit 128
> +EOF
> +$ chmod +x $HOME/git-shell-commands/help
> +----------------
> +
>  SEE ALSO
>  --------
>  ssh(1),
> diff --git a/shell.c b/shell.c
> index 84b237fe..3abc2b84 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -63,10 +63,16 @@ static void cd_to_homedir(void)
>
>  static void run_shell(void)
>  {
> -       int done = 0;
> +       int done = 0, status;
>         static const char *help_argv[] = { HELP_COMMAND, NULL };
>         /* Print help if enabled */
> -       run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> +       status = run_command_v_opt(help_argv, RUN_SILENT_EXEC_FAILURE);
> +       if (!status)
> +               ; /* success */
> +       else if (status == -1 && errno == ENOENT)
> +               ; /* help disabled */
> +       else
> +               exit(status);
>
>         do {
>                 struct strbuf line = STRBUF_INIT;
> --
> 1.8.1.3
>