Before executing svnserve, change the CWD to a readable directory
Summary: Fixes T10941. This avoids a confusing dead end when configuring Subversion hosting, where `svnserve` will fail to execute hooks if the CWD isn't readable by the vcs-user. Test Plan: - Updated and committed in a hosted SVN repository. - Ran some git operations, too. - @dpotter confirmed this locally in T10941. Reviewers: chad Reviewed By: chad Subscribers: dpotter Maniphest Tasks: T10941 Differential Revision: https://secure.phabricator.com/D15879
This commit is contained in:
		| @@ -24,8 +24,7 @@ final class DiffusionGitCommandEngine | |||||||
|     // really silly, but seems like the least damaging approach to |     // really silly, but seems like the least damaging approach to | ||||||
|     // mitigating the issue. |     // mitigating the issue. | ||||||
|  |  | ||||||
|     $root = dirname(phutil_get_library_root('phabricator')); |     $env['HOME'] = PhabricatorEnv::getEmptyCWD(); | ||||||
|     $env['HOME'] = $root.'/support/empty/'; |  | ||||||
|  |  | ||||||
|     if ($this->isAnySSHProtocol()) { |     if ($this->isAnySSHProtocol()) { | ||||||
|       $env['GIT_SSH'] = $this->getSSHWrapper(); |       $env['GIT_SSH'] = $this->getSSHWrapper(); | ||||||
|   | |||||||
| @@ -148,15 +148,25 @@ final class DiffusionSubversionServeSSHWorkflow | |||||||
|     if ($this->shouldProxy()) { |     if ($this->shouldProxy()) { | ||||||
|       $command = $this->getProxyCommand(); |       $command = $this->getProxyCommand(); | ||||||
|       $this->isProxying = true; |       $this->isProxying = true; | ||||||
|  |       $cwd = null; | ||||||
|     } else { |     } else { | ||||||
|       $command = csprintf( |       $command = csprintf( | ||||||
|         'svnserve -t --tunnel-user=%s', |         'svnserve -t --tunnel-user=%s', | ||||||
|         $this->getUser()->getUsername()); |         $this->getUser()->getUsername()); | ||||||
|  |       $cwd = PhabricatorEnv::getEmptyCWD(); | ||||||
|     } |     } | ||||||
|  |  | ||||||
|     $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command); |     $command = PhabricatorDaemon::sudoCommandAsDaemonUser($command); | ||||||
|     $future = new ExecFuture('%C', $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->inProtocol = new DiffusionSubversionWireProtocol(); | ||||||
|     $this->outProtocol = new DiffusionSubversionWireProtocol(); |     $this->outProtocol = new DiffusionSubversionWireProtocol(); | ||||||
|  |  | ||||||
|   | |||||||
							
								
								
									
										17
									
								
								src/infrastructure/env/PhabricatorEnv.php
									
									
									
									
										vendored
									
									
								
							
							
						
						
									
										17
									
								
								src/infrastructure/env/PhabricatorEnv.php
									
									
									
									
										vendored
									
									
								
							| @@ -877,4 +877,21 @@ final class PhabricatorEnv extends Phobject { | |||||||
|     umask(022); |     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/'; | ||||||
|  |   } | ||||||
|  |  | ||||||
|  |  | ||||||
| } | } | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 epriestley
					epriestley