Although I've always been aware of the design intent behind empty hook
script environments, I'll echo Tim's complaint that it's sometimes
inconvenient. The problem most commonly crops up in svn+ssh:// or
file:// deployments where you want to run some action with user
credentials: updating a bug database, updating a shared read-only
working copy, updating a snapshot of the repository, etc.. If the
credentials are pointed to by environment variables (e.g. Kerberos
tickets), then the operation fails. If they are pointed to by other
means (e.g. AFS tokens), then the operation may succeed anyway.
In some cases it may be possible to perform the action with server host
credentials instead, but that is not always a good option.
The design intent behind clearing the environment has two bases:
security and consistency. In evaluating the security basis, you have to
consider the type of deployment:
* http:// and svn:// access, where the client has little or no control
over the environment. In this case propagating the environment carries
no risk because the environment is controlled by the admin.
* file:// and svn+ssh:// access where the calling user has
unrestricted shell priveleges and there is no setuid or setgid bit on
the svn/svnserve binary. In this case propagating the environment
carries no risk because the svn code is not executing with elevated
privilege.
* file:// and svn+ssh:// access where the calling user has restricted
shell access (can only run svn/svnserve for some reason) or there is a
setuid or setgid bit on the svn or svnserve binary. In this case
propagating the environment could open the door to unintended access.
It might be reasonable to have said from the start, "if you're in the
third situation, then your hook scripts should clear their own
environments," but we can't start saying that in release 1.7. We can
detect a setuid or setgid bit, but we cannot detect a restricted shell
situation (such as when .ssh/authorized-keys contains a "command"
directive), so we can't really intuit when it's safe to propagate the
environment.
It would be reasonable to make this controlled by repository
configuration, if there's a convenient place to put that bit.
Received on 2010-03-24 13:18:13 CET