Support serving SVN repositories over SSH

Summary:
Ref T2230. The SVN protocol has a sensible protocol format with a good spec here:

http://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_ra_svn/protocol

Particularly, compare this statement to the clown show that is the Mercurial wire protocol:

> It is possible to parse an item without knowing its type in advance.

WHAT A REASONABLE STATEMENT TO BE ABLE TO MAKE ABOUT A WIRE PROTOCOL

Although it makes substantially more sense than Mercurial, it's much heavier-weight than the Git or Mercurial protocols, since it isn't distributed.

It's also not possible to figure out if a request is a write request (or even which repository it is against) without proxying some of the protocol frames. Finally, several protocol commands embed repository URLs, and we need to reach into the protocol and translate them.

Test Plan: Ran various SVN commands over SSH (`svn log`, `svn up`, `svn commit`, etc).

Reviewers: btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2230

Differential Revision: https://secure.phabricator.com/D7556

scripts/ssh/ssh-exec.php

@@ -61,6 +61,7 @@ try {
 
   $workflows = array(
     new ConduitSSHWorkflow(),
+    new DiffusionSSHSubversionServeWorkflow(),
     new DiffusionSSHMercurialServeWorkflow(),
     new DiffusionSSHGitUploadPackWorkflow(),
     new DiffusionSSHGitReceivePackWorkflow(),

src/__phutil_library_map__.php

@@ -547,10 +547,14 @@ phutil_register_library_map(array(
     'DiffusionSSHMercurialWireClientProtocolChannel' => 'applications/diffusion/ssh/DiffusionSSHMercurialWireClientProtocolChannel.php',
     'DiffusionSSHMercurialWireTestCase' => 'applications/diffusion/ssh/__tests__/DiffusionSSHMercurialWireTestCase.php',
     'DiffusionSSHMercurialWorkflow' => 'applications/diffusion/ssh/DiffusionSSHMercurialWorkflow.php',
+    'DiffusionSSHSubversionServeWorkflow' => 'applications/diffusion/ssh/DiffusionSSHSubversionServeWorkflow.php',
+    'DiffusionSSHSubversionWorkflow' => 'applications/diffusion/ssh/DiffusionSSHSubversionWorkflow.php',
     'DiffusionSSHWorkflow' => 'applications/diffusion/ssh/DiffusionSSHWorkflow.php',
     'DiffusionServeController' => 'applications/diffusion/controller/DiffusionServeController.php',
     'DiffusionSetPasswordPanel' => 'applications/diffusion/panel/DiffusionSetPasswordPanel.php',
     'DiffusionSetupException' => 'applications/diffusion/exception/DiffusionSetupException.php',
+    'DiffusionSubversionWireProtocol' => 'applications/diffusion/protocol/DiffusionSubversionWireProtocol.php',
+    'DiffusionSubversionWireProtocolTestCase' => 'applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php',
     'DiffusionSvnCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionSvnCommitParentsQuery.php',
     'DiffusionSvnFileContentQuery' => 'applications/diffusion/query/filecontent/DiffusionSvnFileContentQuery.php',
     'DiffusionSvnRawDiffQuery' => 'applications/diffusion/query/rawdiff/DiffusionSvnRawDiffQuery.php',
@@ -2816,10 +2820,14 @@ phutil_register_library_map(array(
     'DiffusionSSHMercurialWireClientProtocolChannel' => 'PhutilProtocolChannel',
     'DiffusionSSHMercurialWireTestCase' => 'PhabricatorTestCase',
     'DiffusionSSHMercurialWorkflow' => 'DiffusionSSHWorkflow',
+    'DiffusionSSHSubversionServeWorkflow' => 'DiffusionSSHSubversionWorkflow',
+    'DiffusionSSHSubversionWorkflow' => 'DiffusionSSHWorkflow',
     'DiffusionSSHWorkflow' => 'PhabricatorSSHWorkflow',
     'DiffusionServeController' => 'DiffusionController',
     'DiffusionSetPasswordPanel' => 'PhabricatorSettingsPanel',
     'DiffusionSetupException' => 'AphrontUsageException',
+    'DiffusionSubversionWireProtocol' => 'Phobject',
+    'DiffusionSubversionWireProtocolTestCase' => 'PhabricatorTestCase',
     'DiffusionSvnCommitParentsQuery' => 'DiffusionCommitParentsQuery',
     'DiffusionSvnFileContentQuery' => 'DiffusionFileContentQuery',
     'DiffusionSvnRawDiffQuery' => 'DiffusionRawDiffQuery',

src/applications/diffusion/protocol/DiffusionSubversionWireProtocol.php

@@ -0,0 +1,188 @@
+<?php
+
+final class DiffusionSubversionWireProtocol extends Phobject {
+
+  private $buffer = '';
+  private $state = 'item';
+  private $expectBytes = 0;
+  private $byteBuffer = '';
+  private $stack = array();
+  private $list = array();
+  private $raw = '';
+
+  private function pushList() {
+    $this->stack[] = $this->list;
+    $this->list = array();
+  }
+
+  private function popList() {
+    $list = $this->list;
+    $this->list = array_pop($this->stack);
+    return $list;
+  }
+
+  private function pushItem($item, $type) {
+    $this->list[] = array(
+      'type' => $type,
+      'value' => $item,
+    );
+  }
+
+  public function writeData($data) {
+    $this->buffer .= $data;
+
+    $messages = array();
+    while (true) {
+      if ($this->state == 'item') {
+        $match = null;
+        $result = null;
+        $buf = $this->buffer;
+        if (preg_match('/^([a-z][a-z0-9-]*)\s/i', $buf, $match)) {
+          $this->pushItem($match[1], 'word');
+        } else if (preg_match('/^(\d+)\s/', $buf, $match)) {
+          $this->pushItem((int)$match[1], 'number');
+        } else if (preg_match('/^(\d+):/', $buf, $match)) {
+          // NOTE: The "+ 1" includes the space after the string.
+          $this->expectBytes = (int)$match[1] + 1;
+          $this->state = 'bytes';
+        } else if (preg_match('/^(\\()\s/', $buf, $match)) {
+          $this->pushList();
+        } else if (preg_match('/^(\\))\s/', $buf, $match)) {
+          $list = $this->popList();
+          if ($this->stack) {
+            $this->pushItem($list, 'list');
+          } else {
+            $result = $list;
+          }
+        } else {
+          $match = false;
+        }
+
+        if ($match !== false) {
+          $this->raw .= substr($this->buffer, 0, strlen($match[0]));
+          $this->buffer = substr($this->buffer, strlen($match[0]));
+
+          if ($result !== null) {
+            $messages[] = array(
+              'structure' => $list,
+              'raw' => $this->raw,
+            );
+            $this->raw = '';
+          }
+        } else {
+          // No matches yet, wait for more data.
+          break;
+        }
+      } else if ($this->state == 'bytes') {
+        $new_data = substr($this->buffer, 0, $this->expectBytes);
+        $this->buffer = substr($this->buffer, strlen($new_data));
+
+        $this->expectBytes -= strlen($new_data);
+        $this->raw .= $new_data;
+        $this->byteBuffer .= $new_data;
+
+        if (!$this->expectBytes) {
+          $this->state = 'byte-space';
+          // Strip off the terminal space.
+          $this->pushItem(substr($this->byteBuffer, 0, -1), 'string');
+          $this->byteBuffer = '';
+          $this->state = 'item';
+        }
+      } else {
+        throw new Exception("Invalid state '{$this->state}'!");
+      }
+    }
+
+    return $messages;
+  }
+
+  /**
+   * Convert a parsed command struct into a wire protocol string.
+   */
+  public function serializeStruct(array $struct) {
+    $out = array();
+
+    $out[] = '( ';
+    foreach ($struct as $item) {
+      $value = $item['value'];
+      $type = $item['type'];
+      switch ($type) {
+        case 'word':
+          $out[] = $value;
+          break;
+        case 'number':
+          $out[] = $value;
+          break;
+        case 'string':
+          $out[] = strlen($value).':'.$value;
+          break;
+        case 'list':
+          $out[] = self::serializeStruct($value);
+          break;
+        default:
+          throw new Exception("Unknown SVN wire protocol structure '{$type}'!");
+      }
+      if ($type != 'list') {
+        $out[] = ' ';
+      }
+    }
+    $out[] = ') ';
+
+    return implode('', $out);
+  }
+
+  public function isReadOnlyCommand(array $struct) {
+    if (empty($struct[0]['type']) || ($struct[0]['type'] != 'word')) {
+      // This isn't what we expect; fail defensively.
+      throw new Exception(
+        pht("Unexpected command structure, expected '( word ... )'."));
+    }
+
+    switch ($struct[0]['value']) {
+      // Authentication command set.
+      case 'EXTERNAL':
+
+      // The "Main" command set. Some of the commands in this command set are
+      // mutation commands, and are omitted from this list.
+      case 'reparent':
+      case 'get-latest-rev':
+      case 'get-dated-rev':
+      case 'rev-proplist':
+      case 'rev-prop':
+      case 'get-file':
+      case 'get-dir':
+      case 'check-path':
+      case 'stat':
+      case 'update':
+      case 'get-mergeinfo':
+      case 'switch':
+      case 'status':
+      case 'diff':
+      case 'log':
+      case 'get-file-revs':
+      case 'get-locations':
+
+      // The "Report" command set. These are not actually mutation
+      // operations, they just define a request for information.
+      case 'set-path':
+      case 'delete-path':
+      case 'link-path':
+      case 'finish-report':
+      case 'abort-report':
+
+      // These are used to report command results.
+      case 'success':
+      case 'failure':
+
+        // If we get here, we've matched some known read-only command.
+        return true;
+      default:
+        // Anything else isn't a known read-only command, so require write
+        // access to use it.
+        break;
+    }
+
+    return false;
+  }
+
+}

src/applications/diffusion/protocol/__tests__/DiffusionSubversionWireProtocolTestCase.php

@@ -0,0 +1,80 @@
+<?php
+
+final class DiffusionSubversionWireProtocolTestCase
+  extends PhabricatorTestCase {
+
+  public function testSubversionWireProtocolParser() {
+    $this->assertSameSubversionMessages(
+      '( ) ',
+      array(
+        array(
+        ),
+      ));
+
+    $this->assertSameSubversionMessages(
+      '( duck 5:quack 42 ( item1 item2 ) ) ',
+      array(
+        array(
+          array(
+            'type' => 'word',
+            'value' => 'duck',
+          ),
+          array(
+            'type' => 'string',
+            'value' => 'quack',
+          ),
+          array(
+            'type' => 'number',
+            'value' => 42,
+          ),
+          array(
+            'type' => 'list',
+            'value' => array(
+              array(
+                'type' => 'word',
+                'value' => 'item1',
+              ),
+              array(
+                'type' => 'word',
+                'value' => 'item2',
+              ),
+            ),
+          ),
+        ),
+      ));
+
+    $this->assertSameSubversionMessages(
+      '( msg1 ) ( msg2 ) ',
+      array(
+        array(
+          array(
+            'type' => 'word',
+            'value' => 'msg1',
+          ),
+        ),
+        array(
+          array(
+            'type' => 'word',
+            'value' => 'msg2',
+          ),
+        ),
+      ));
+  }
+
+  private function assertSameSubversionMessages($string, array $structs) {
+    $proto = new DiffusionSubversionWireProtocol();
+
+    // Verify that the wire message parses into the structs.
+    $messages = $proto->writeData($string);
+    $messages = ipull($messages, 'structure');
+    $this->assertEqual($structs, $messages, 'parse<'.$string.'>');
+
+    // Verify that the structs serialize into the wire message.
+    $serial = array();
+    foreach ($structs as $struct) {
+      $serial[] = $proto->serializeStruct($struct);
+    }
+    $serial = implode('', $serial);
+    $this->assertEqual($string, $serial, 'serialize<'.$string.'>');
+  }
+}

src/applications/diffusion/ssh/DiffusionSSHGitReceivePackWorkflow.php

@@ -14,13 +14,10 @@ final class DiffusionSSHGitReceivePackWorkflow
       ));
   }
 
-  public function getRequestPath() {
+  protected function executeRepositoryOperations() {
     $args = $this->getArgs();
-    return head($args->getArg('dir'));
+    $path = head($args->getArg('dir'));
-  }
+    $repository = $this->loadRepository($path);
-
-  protected function executeRepositoryOperations(
-    PhabricatorRepository $repository) {
 
     // This is a write, and must have write access.
     $this->requireWriteAccess();
@@ -28,6 +25,7 @@ final class DiffusionSSHGitReceivePackWorkflow
     $future = new ExecFuture(
       'git-receive-pack %s',
       $repository->getLocalPath());
+
     $err = $this->newPassthruCommand()
       ->setIOChannel($this->getIOChannel())
       ->setCommandChannelFromExecFuture($future)

src/applications/diffusion/ssh/DiffusionSSHGitUploadPackWorkflow.php

@@ -14,13 +14,10 @@ final class DiffusionSSHGitUploadPackWorkflow
       ));
   }
 
-  public function getRequestPath() {
+  protected function executeRepositoryOperations() {
     $args = $this->getArgs();
-    return head($args->getArg('dir'));
+    $path = head($args->getArg('dir'));
-  }
+    $repository = $this->loadRepository($path);
-
-  protected function executeRepositoryOperations(
-    PhabricatorRepository $repository) {
 
     $future = new ExecFuture('git-upload-pack %s', $repository->getLocalPath());
 

src/applications/diffusion/ssh/DiffusionSSHMercurialServeWorkflow.php

@@ -24,12 +24,10 @@ final class DiffusionSSHMercurialServeWorkflow
       ));
   }
 
-  public function getRequestPath() {
+  protected function executeRepositoryOperations() {
-    return $this->getArgs()->getArg('repository');
+    $args = $this->getArgs();
-  }
+    $path = $args->getArg('repository');
-
+    $repository = $this->loadRepository($path);
-  protected function executeRepositoryOperations(
-    PhabricatorRepository $repository) {
 
     $args = $this->getArgs();
 

src/applications/diffusion/ssh/DiffusionSSHSubversionServeWorkflow.php

@@ -0,0 +1,259 @@
+<?php
+
+/**
+ * This protocol has a good spec here:
+ *
+ *   http://svn.apache.org/repos/asf/subversion/trunk/subversion/libsvn_ra_svn/protocol
+ *
+ */
+final class DiffusionSSHSubversionServeWorkflow
+  extends DiffusionSSHSubversionWorkflow {
+
+  private $didSeeWrite;
+
+  private $inProtocol;
+  private $outProtocol;
+
+  private $inSeenGreeting;
+
+  private $outPhaseCount = 0;
+
+  private $internalBaseURI;
+  private $externalBaseURI;
+
+  public function didConstruct() {
+    $this->setName('svnserve');
+    $this->setArguments(
+      array(
+        array(
+          'name' => 'tunnel',
+          'short' => 't',
+        ),
+      ));
+  }
+
+  protected function executeRepositoryOperations() {
+    $args = $this->getArgs();
+    if (!$args->getArg('tunnel')) {
+      throw new Exception("Expected `svnserve -t`!");
+    }
+
+    $future = new ExecFuture('svnserve -t');
+
+    $this->inProtocol = new DiffusionSubversionWireProtocol();
+    $this->outProtocol = new DiffusionSubversionWireProtocol();
+
+    $err = id($this->newPassthruCommand())
+      ->setIOChannel($this->getIOChannel())
+      ->setCommandChannelFromExecFuture($future)
+      ->setWillWriteCallback(array($this, 'willWriteMessageCallback'))
+      ->setWillReadCallback(array($this, 'willReadMessageCallback'))
+      ->execute();
+
+    if (!$err && $this->didSeeWrite) {
+      $this->getRepository()->writeStatusMessage(
+        PhabricatorRepositoryStatusMessage::TYPE_NEEDS_UPDATE,
+        PhabricatorRepositoryStatusMessage::CODE_OKAY);
+    }
+
+    return $err;
+  }
+
+  public function willWriteMessageCallback(
+    PhabricatorSSHPassthruCommand $command,
+    $message) {
+
+    $proto = $this->inProtocol;
+    $messages = $proto->writeData($message);
+
+    $result = array();
+    foreach ($messages as $message) {
+      $message_raw = $message['raw'];
+      $struct = $message['structure'];
+
+      if (!$this->inSeenGreeting) {
+        $this->inSeenGreeting = true;
+
+        // The first message the client sends looks like:
+        //
+        //   ( version ( cap1 ... ) url ... )
+        //
+        // We want to grab the URL, load the repository, make sure it exists and
+        // is accessible, and then replace it with the location of the
+        // repository on disk.
+
+        $uri = $struct[2]['value'];
+        $struct[2]['value'] = $this->makeInternalURI($uri);
+
+        $message_raw = $proto->serializeStruct($struct);
+      } else if (isset($struct[0]) && $struct[0]['type'] == 'word') {
+
+        if (!$proto->isReadOnlyCommand($struct)) {
+          $this->didSeeWrite = true;
+          $this->requireWriteAccess($struct[0]['value']);
+        }
+
+        // Several other commands also pass in URLs. We need to translate
+        // all of these into the internal representation; this also makes sure
+        // they're valid and accessible.
+
+        switch ($struct[0]['value']) {
+          case 'reparent':
+            // ( reparent ( url ) )
+            $struct[1]['value'][0]['value'] = $this->makeInternalURI(
+              $struct[1]['value'][0]['value']);
+            $message_raw = $proto->serializeStruct($struct);
+            break;
+          case 'switch':
+            // ( switch ( ( rev ) target recurse url ... ) )
+            $struct[1]['value'][3]['value'] = $this->makeInternalURI(
+              $struct[1]['value'][3]['value']);
+            $message_raw = $proto->serializeStruct($struct);
+            break;
+          case 'diff':
+            // ( diff ( ( rev ) target recurse ignore-ancestry url ... ) )
+            $struct[1]['value'][4]['value'] = $this->makeInternalURI(
+              $struct[1]['value'][4]['value']);
+            $message_raw = $proto->serializeStruct($struct);
+            break;
+        }
+      }
+
+      $result[] = $message_raw;
+    }
+
+    if (!$result) {
+      return null;
+    }
+
+    return implode('', $result);
+  }
+
+  public function willReadMessageCallback(
+    PhabricatorSSHPassthruCommand $command,
+    $message) {
+
+    $proto = $this->outProtocol;
+    $messages = $proto->writeData($message);
+
+    $result = array();
+    foreach ($messages as $message) {
+      $message_raw = $message['raw'];
+      $struct = $message['structure'];
+
+      if (isset($struct[0]) && ($struct[0]['type'] == 'word')) {
+
+        if ($struct[0]['value'] == 'success') {
+          switch ($this->outPhaseCount) {
+            case 0:
+              // This is the "greeting", which announces capabilities.
+              break;
+            case 1:
+              // This responds to the client greeting, and announces auth.
+              break;
+            case 2:
+              // This responds to auth, which should be trivial over SSH.
+              break;
+            case 3:
+              // This contains the URI of the repository. We need to edit it;
+              // if it does not match what the client requested it will reject
+              // the response.
+              $struct[1]['value'][1]['value'] = $this->makeExternalURI(
+                $struct[1]['value'][1]['value']);
+              $message_raw = $proto->serializeStruct($struct);
+              break;
+            default:
+              // We don't care about other protocol frames.
+              break;
+          }
+
+          $this->outPhaseCount++;
+        } else if ($struct[0]['value'] == 'failure') {
+          // Find any error messages which include the internal URI, and
+          // replace the text with the external URI.
+          foreach ($struct[1]['value'] as $key => $error) {
+            $code = $error['value'][0]['value'];
+            $message = $error['value'][1]['value'];
+
+            $message = str_replace(
+              $this->internalBaseURI,
+              $this->externalBaseURI,
+              $message);
+
+            // Derp derp derp derp derp. The structure looks like this:
+            //   ( failure ( ( code message ... ) ... ) )
+            $struct[1]['value'][$key]['value'][1]['value'] = $message;
+          }
+          $message_raw = $proto->serializeStruct($struct);
+        }
+
+      }
+
+      $result[] = $message_raw;
+    }
+
+    if (!$result) {
+      return null;
+    }
+
+    return implode('', $result);
+  }
+
+  private function makeInternalURI($uri_string) {
+    $uri = new PhutilURI($uri_string);
+
+    $proto = $uri->getProtocol();
+    if ($proto !== 'svn+ssh') {
+      throw new Exception(
+        pht(
+          'Protocol for URI "%s" MUST be "svn+ssh".',
+          $uri_string));
+    }
+
+    $path = $uri->getPath();
+
+    // Subversion presumably deals with this, but make sure there's nothing
+    // skethcy going on with the URI.
+    if (preg_match('(/\\.\\./)', $path)) {
+      throw new Exception(
+        pht(
+          'String "/../" is invalid in path specification "%s".',
+          $uri_string));
+    }
+
+    $repository = $this->loadRepository($path);
+
+    $path = preg_replace(
+      '(^/diffusion/[A-Z]+)',
+      rtrim($repository->getLocalPath(), '/'),
+      $path);
+
+    if (preg_match('(^/diffusion/[A-Z]+/$)', $path)) {
+      $path = rtrim($path, '/');
+    }
+
+    $uri->setPath($path);
+
+    // If this is happening during the handshake, these are the base URIs for
+    // the request.
+    if ($this->externalBaseURI === null) {
+      $pre = (string)id(clone $uri)->setPath('');
+      $this->externalBaseURI = $pre.'/diffusion/'.$repository->getCallsign();
+      $this->internalBaseURI = $pre.rtrim($repository->getLocalPath(), '/');
+    }
+
+    return (string)$uri;
+  }
+
+  private function makeExternalURI($uri) {
+    $internal = $this->internalBaseURI;
+    $external = $this->externalBaseURI;
+
+    if (strncmp($uri, $internal, strlen($internal)) === 0) {
+      $uri = $external.substr($uri, strlen($internal));
+    }
+
+    return $uri;
+  }
+
+}

src/applications/diffusion/ssh/DiffusionSSHSubversionWorkflow.php

@@ -0,0 +1,10 @@
+<?php
+
+abstract class DiffusionSSHSubversionWorkflow extends DiffusionSSHWorkflow {
+
+  protected function writeError($message) {
+    // Subversion assumes we'll add our own newlines.
+    return parent::writeError($message."\n");
+  }
+
+}

src/applications/diffusion/ssh/DiffusionSSHWorkflow.php

@@ -7,6 +7,9 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
   private $hasWriteAccess;
 
   public function getRepository() {
+    if (!$this->repository) {
+      throw new Exception("Call loadRepository() before getRepository()!");
+    }
     return $this->repository;
   }
 
@@ -14,9 +17,7 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
     return $this->args;
   }
 
-  abstract protected function getRequestPath();
+  abstract protected function executeRepositoryOperations();
-  abstract protected function executeRepositoryOperations(
-    PhabricatorRepository $repository);
 
   protected function writeError($message) {
     $this->getErrorChannel()->write($message);
@@ -27,18 +28,15 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
     $this->args = $args;
 
     try {
-      $repository = $this->loadRepository();
+      return $this->executeRepositoryOperations();
-      $this->repository = $repository;
-      return $this->executeRepositoryOperations($repository);
     } catch (Exception $ex) {
       $this->writeError(get_class($ex).': '.$ex->getMessage());
       return 1;
     }
   }
 
-  private function loadRepository() {
+  protected function loadRepository($path) {
     $viewer = $this->getUser();
-    $path = $this->getRequestPath();
 
     $regex = '@^/?diffusion/(?P<callsign>[A-Z]+)(?:/|$)@';
     $matches = null;
@@ -74,10 +72,12 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
           pht('This repository is not available over SSH.'));
     }
 
+    $this->repository = $repository;
+
     return $repository;
   }
 
-  protected function requireWriteAccess() {
+  protected function requireWriteAccess($protocol_command = null) {
     if ($this->hasWriteAccess === true) {
       return;
     }
@@ -87,8 +87,16 @@ abstract class DiffusionSSHWorkflow extends PhabricatorSSHWorkflow {
 
     switch ($repository->getServeOverSSH()) {
       case PhabricatorRepository::SERVE_READONLY:
+        if ($protocol_command !== null) {
+          throw new Exception(
+            pht(
+              'This repository is read-only over SSH (tried to execute '.
+              'protocol command "%s").',
+              $protocol_command));
+        } else {
           throw new Exception(
             pht('This repository is read-only over SSH.'));
+        }
         break;
       case PhabricatorRepository::SERVE_READWRITE:
         $can_push = PhabricatorPolicyFilter::hasCapability(

src/infrastructure/ssh/PhabricatorSSHPassthruCommand.php

@@ -139,6 +139,12 @@ final class PhabricatorSSHPassthruCommand extends Phobject {
       if ($done) {
         break;
       }
+
+      // If the client has disconnected, kill the subprocess and bail.
+      if (!$io_channel->isOpenForWriting()) {
+        $this->execFuture->resolveKill();
+        break;
+      }
     }
 
     list($err) = $this->execFuture->resolve();