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

[Johannes Löthberg]

On 23/06, Lukas Fleischer wrote:
>On Tue, 23 Jun 2015 at 00:00:18, Johannes Löthberg wrote:
>> Also add an utility function for formatting the ForceCommand
>>
>
>Missing punctuation :)
>

Ah, silly me.

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

Yeah, I'll see about replacing the other uses.

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

Thing is that the command will actually be

  sh -c 'FOO=bar echo $FOO'

which won't work. Either

  sh -c 'FOO=bar && echo $FOO'

or

  sh -c 'FOO=bar; echo $FOO'

is needed.

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

Yes, that's fair, will add that.

-- 
Sincerely,
  Johannes Löthberg
  PGP Key ID: 0x50FB9B273A9D0BB5
  https://theos.kyriasis.com/~kyrias/
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 1495 bytes
Desc: not available
URL: <https://lists.archlinux.org/pipermail/aur-dev/attachments/20150623/7d53016d/attachment.asc>