[PATCHv4 2/2] Helper script to interface to gitolite

[Ben Boeckel]

On Wed, Oct 31, 2012 at 23:03:01 -0400, Jamie Couture wrote:
> What about users that have installed gitolite via their distro's
> package manager, as opposed to local install?

FreeBSD installs to /usr/local, so the path is fine for it.

> I do not think the script should assume to know where gitolite lives.
> 
> This might be agnostic:
> prog="$(which gitolite)"

It could also be configured with $PREFIX as part of the build. Any
hard-coded path is going to be wrong somewhere, so I think this might be
for distros to decide (does cgit even install contrib/ stuff by
default?). However...

> But that could lead to PATH abuse; potential security problems.

if you've injected something into $PATH as the user running the CGI
program, that's likely a system setup misconfiguration and I doubt
everything down that rabbit hole could be protected against ("Oops,
you're using an Apache setup vulnerable to BEAST; let's mitigate...").
If attackers are setting PATH somehow, there are other issues beyond
cgit's scope. Since cgit doesn't (and if it does, that probably needs to
not happen) touch PATH, it therefore shouldn't be responsible for
securing it.

> Maybe obtaining the value from another environment variable, say in
> your web server's virtual host:
> 
> SetEnv GITOLITE_PROG /path/to/gitolite
> 
> ---
> 
> prog="${GITOLITE_PROG}"

Assuming cgit doesn't touch PATH, this is just as valid an attack target
if PATH is an attack vector. Nothing is gained by making another
environment variable.

> Not sure how much of a crutch that is, but I wouldn't want the helper
> script to assume where gitolite lives; and if any distro were to
> include these scripts people might want it to work out of the box.

The solutions I see are:

  - hook it up to the build system (if contrib is already a part of it);
  - use `which`; or
  - ignore it and just have downstreams patch it for the system paths.

I'm in favor of the second one because it's the simplest solution. (I
don't think another patchset would be required if this is the only
thing; a commit on top of these should be fine; --amend should be fine
as well).

--Ben