diff --git a/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php b/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php index 3458d27ed6..5eae19e030 100644 --- a/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php +++ b/src/applications/diffusion/protocol/DiffusionGitCommandEngine.php @@ -24,8 +24,7 @@ final class DiffusionGitCommandEngine // really silly, but seems like the least damaging approach to // mitigating the issue. - $root = dirname(phutil_get_library_root('phabricator')); - $env['HOME'] = $root.'/support/empty/'; + $env['HOME'] = PhabricatorEnv::getEmptyCWD(); if ($this->isAnySSHProtocol()) { $env['GIT_SSH'] = $this->getSSHWrapper(); diff --git a/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php b/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php index 820a380856..57a44d4707 100644 --- a/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php +++ b/src/applications/diffusion/ssh/DiffusionSubversionServeSSHWorkflow.php @@ -148,15 +148,25 @@ final class DiffusionSubversionServeSSHWorkflow if ($this->shouldProxy()) { $command = $this->getProxyCommand(); $this->isProxying = true; + $cwd = null; } else { $command = csprintf( 'svnserve -t --tunnel-user=%s', $this->getUser()->getUsername()); + $cwd = PhabricatorEnv::getEmptyCWD(); } $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command); $future = new ExecFuture('%C', $command); + // If we're receiving a commit, svnserve will fail to execute the commit + // hook with an unhelpful error if the CWD isn't readable by the user we + // are sudoing to. Switch to a readable, empty CWD before running + // svnserve. See T10941. + if ($cwd !== null) { + $future->setCWD($cwd); + } + $this->inProtocol = new DiffusionSubversionWireProtocol(); $this->outProtocol = new DiffusionSubversionWireProtocol(); diff --git a/src/infrastructure/env/PhabricatorEnv.php b/src/infrastructure/env/PhabricatorEnv.php index 60bd9e4595..dd8c11e629 100644 --- a/src/infrastructure/env/PhabricatorEnv.php +++ b/src/infrastructure/env/PhabricatorEnv.php @@ -877,4 +877,21 @@ final class PhabricatorEnv extends Phobject { umask(022); } + + /** + * Get the path to an empty directory which is readable by all of the system + * user accounts that Phabricator acts as. + * + * In some cases, a binary needs some valid HOME or CWD to continue, but not + * all user accounts have valid home directories and even if they do they + * may not be readable after a `sudo` operation. + * + * @return string Path to an empty directory suitable for use as a CWD. + */ + public static function getEmptyCWD() { + $root = dirname(phutil_get_library_root('phabricator')); + return $root.'/support/empty/'; + } + + }