[aur-dev] [PATCH 1/4] git: Use AUR_USER env var instead of ForceCommand argument

[Lukas Fleischer]

On Tue, 23 Jun 2015 at 00:00:18, Johannes Löthberg wrote:
> Also add an utility function for formatting the ForceCommand
> 

Missing punctuation :)

> Signed-off-by: Johannes Löthberg <johannes at kyriasis.com>
> ---
>  git-interface/git-auth.py  | 18 ++++++++++++++++--
>  git-interface/git-serve.py |  2 +-
>  2 files changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/git-interface/git-auth.py b/git-interface/git-auth.py
> index c9e1f01..af1f8d4 100755
> --- a/git-interface/git-auth.py
> +++ b/git-interface/git-auth.py
> @@ -6,6 +6,16 @@ import os
>  import re
>  import sys
>  
> +
> +def format_command(env_vars, command, ssh_opts, key):
> +    environment = ''
> +    for key, var in env_vars.items():
> +        environment += '{}={} && '.format(key, var)

I like this idea. I noticed three things, though:

1. We do not seem to use format() anywhere else. Maybe use the %
   operator for consistency? But then again, % is obsolete, so we should
   probably rather replace all the existing formatting operations by
   format() (in another patch)...

2. Is there any reason to &&-chain the environment variable assignments?
   Does this even work? When I run

    FOO=bar sh -c 'echo $FOO'

   it prints "bar" as expected, but when I insert "&&" between the first
   two tokens it doesn't.

3. I notice this is not strictly needed now but if we write a helper
   function, it might be a good idea to quote the arguments (e.g. using
   shlex.quote). It is not unlikely that we want to add arguments that
   may contains spaces some day. This is the most security critical part
   of the Git interface. If we do not escape things properly, users
   might be able to execute arbitrary code on the server. We should be
   extra cautious here...

> +
> +    msg = 'command="{}{}",{} {}'.format(environment, command, ssh_opts, key)
> +    return msg
> +
> +
>  config = configparser.RawConfigParser()
>  config.read(os.path.dirname(os.path.realpath(__file__)) + "/../conf/config")
>  
> @@ -40,5 +50,9 @@ user = cur.fetchone()[0]
>  if not re.match(username_regex, user):
>      exit(1)
>  
> -print('command="%s %s",%s %s' % (git_serve_cmd, user, ssh_opts,
> -    keytype + " " + keytext))
> +env_vars = {
> +    'AUR_USER': user,
> +}
> +key = keytype + ' ' + keytext
> +
> +print(format_command(env_vars, git_serve_cmd, ssh_opts, key))
> diff --git a/git-interface/git-serve.py b/git-interface/git-serve.py
> index 26aa02d..6f521cc 100755
> --- a/git-interface/git-serve.py
> +++ b/git-interface/git-serve.py
> @@ -106,7 +106,7 @@ def die(msg):
>  def die_with_help(msg):
>      die(msg + "\nTry `%s help` for a list of commands." % (ssh_cmdline))
>  
> -user = sys.argv[1]
> +user = os.environ.get("AUR_USER")
>  cmd = os.environ.get("SSH_ORIGINAL_COMMAND")
>  if not cmd:
>      die_with_help("Interactive shell is disabled.")
> -- 
> 2.4.4