[Tickets #5051] SVN integration is not robust and has poor error reporting

[bugs@bugs.horde.org]

DO NOT REPLY TO THIS MESSAGE. THIS EMAIL ADDRESS IS NOT MONITORED.

Ticket URL: http://bugs.horde.org/ticket/?id=5051
-----------------------------------------------------------------------
 Ticket             | 5051
 Created By         | meyer at mesw.de
 Summary            | SVN integration is not robust and has poor error reporting
 Queue              | Horde Framework Packages
 Version            | HEAD
 Type               | Bug
 State              | Unconfirmed
 Priority           | 1. Low
 Owners             | 
+New Attachment     | vc.patch
-----------------------------------------------------------------------


meyer at mesw.de (2007-02-28 15:10) wrote:

Judging from posts on the mailing lists, it is common that a user installs
Chora and configures it to use SVN but the content of the repositories are
not shown, yet there are no error messages and no messages in the logs and
there does not seem to be an apparent misconfiguration.

The attached patch tries to improve this situation by making the following
changes:

(1)

In VC::getPath(), the path of the SVN (or any other) binary is only
returned if php determines it is executable. This was introduced without
explanation in log message or comment in version 1.7:

http://cvs.horde.org/diff.php?r1=1.6&r2=1.7&f=framework%2FVC%2FVC.php

This is wrong for two reasons:

First, when a PHP basedir restriction to the htdocs/ directory is active
(common on many webservers), PHP's built-in function "is_executable"
cannot access files in, say, /usr/bin although it can execute this file.
Infact, the only way to see if the file can be executed is to try to
execute it. Thus, the use of "is_executable" here forces the user to
extend the basedir restriction to /usr/bin, which is a bad idea from a
security point of view.

Second, the error return value "false" is never checked within "svn.php"
and therefore wrong commands will be executed. E.g. when the main module
list of the SVN repository is supposed to be shown, Chora will build a
string of the form "/usr/bin/svn ls /path/to/repository". If
"/usr/bin/svn" is not found, "ls /path/to/repository" will be executed
instead. This will succeed (=> no error message), but the returned output
will not match the expected output (=> modules list will be empty). Thus,
the user has no chance to find out what is wrong than to debug the code.

Therefore the patch restores the code which was present in rev 1.6 of
VC.php. This will always try to execute the SVN binary. If the SVN binary
is not found, a sensible error message will be shown.

(2)

A common configuration on web servers (e.g. the default configuration on
SUSE 9.x) is that the apache process runs as root but the web pages are
handled by the "wwwroot" user which has low privileges. Since the apache
process is started as root, its HOME dir will point to "/root". In this
case, svn, when started by the "wwwroot" user will look for the presence
of "/root/.svn" and fail because it obviously is denied access. This
behaviour can be prevented by explicitely setting the "--config-dir"
directive to a directory the current user has access to (say, something
like "/tmp"). The attached patch automates this behaviour.