Re: [PATCH/RFC 1/4] Allow creation of arbitrary git-shell commands

[Junio C Hamano <gitster@pobox.com>]

Greg Brockman <gdb@MIT.EDU> writes:

> This provides a mechanism for the server to expose custom
> functionality to clients.  My particular use case is that I would like
> a way of discovering all repositories available for cloning.  A
> client that clones via
>   git clone user@example.com
> can invoke a command by
>   ssh user@example.com $command

Please have a blank line above and below sample command display like these
for readability.

> Signed-off-by: Greg Brockman <gdb@mit.edu>
> ---
>  shell.c |   16 ++++++++++++++++
>  1 files changed, 16 insertions(+), 0 deletions(-)
>
> diff --git a/shell.c b/shell.c
> index e4864e0..3fee0ed 100644
> --- a/shell.c
> +++ b/shell.c
> @@ -3,6 +3,8 @@
>  #include "exec_cmd.h"
>  #include "strbuf.h"
>  
> +#define COMMAND_DIR "git-shell-commands"
> +
>  static int do_generic_cmd(const char *me, char *arg)
>  {
>  	const char *my_argv[4];
> @@ -33,6 +35,12 @@ static int do_cvs_cmd(const char *me, char *arg)
>  	return execv_git_cmd(cvsserver_argv);
>  }
>  
> +static int is_valid_cmd_name(const char *cmd)
> +{
> +	/* Test command contains no . or / characters */
> +	return cmd[strcspn(cmd, "./")] == '\0';
> +}
> +
>  
>  static struct commands {
>  	const char *name;
> @@ -99,5 +107,13 @@ int main(int argc, char **argv)
>  		}
>  		exit(cmd->exec(cmd->name, arg));
>  	}
> +
> +	/* Shell should be spawned with cwd in the git user's home directory */
> +	if (chdir(COMMAND_DIR))
> +		die("unrecognized command '%s'", prog);

Hmm, could you justify "should be" above please?

An example would be "All of the custom commands I wrote to give added
features to users at my installation wanted to be in that directory, not
at the user's home directory, as they mostly operated on files in that
directory", but please do not make me (or other reviewers) guess why.

What I am getting at is that it may be more natural and useful to run
these custom commands in the user's $HOME directory---you would need to
make sure that execl() finds the command you get from the request, perhaps
by prefixing COMMAND_DIR / to the command name, though.

> +	if (is_valid_cmd_name(prog))
> +		execl(prog, prog, (char *) NULL);
> +
>  	die("unrecognized command '%s'", prog);
>  }