From 2dc8065d114335c957771660ced4090c0f3cddd7 Mon Sep 17 00:00:00 2001 From: epriestley Date: Wed, 13 Nov 2013 11:26:22 -0800 Subject: [PATCH] Prevent Repository local path edit from the web UI Summary: Ref T4039. This fixes an issue where a user with the ability to create repositories could view repositories he is otherwise not permitted to see, by following these steps: - Suppose you want to see repository "A". - Create a repository with the same VCS, called "B". - Edit the local path, changing "/var/repo/B" to "/var/repo/A". - Now it points at a working copy of a repository you can't see. - Although you won't be able to make it through discovery (the pull will fail with the wrong credentials), you can read some information out of the repository directly through the Diffusion UI, probably? I'm not sure this was really practical to execute since there are a bunch of sanity checks along most/all of the major pathways, but lock it down since normal users shouldn't be editing it anyway. In the best case, this would make a mess. Test Plan: {F81391} Reviewers: btrahan Reviewed By: btrahan CC: aran Maniphest Tasks: T4039 Differential Revision: https://secure.phabricator.com/D7580 --- ...DiffusionRepositoryEditLocalController.php | 48 ++++--------------- 1 file changed, 8 insertions(+), 40 deletions(-) diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditLocalController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditLocalController.php index 33c004c847..225ca9bd36 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditLocalController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditLocalController.php @@ -26,41 +26,8 @@ final class DiffusionRepositoryEditLocalController $edit_uri = $this->getRepositoryControllerURI($repository, 'edit/'); $v_local = $repository->getHumanReadableDetail('local-path'); - $e_local = true; $errors = array(); - if ($request->isFormPost()) { - $v_local = $request->getStr('local'); - - if (!strlen($v_local)) { - $e_local = pht('Required'); - $errors[] = pht('You must specify a local path.'); - } - - if (!$errors) { - $xactions = array(); - $template = id(new PhabricatorRepositoryTransaction()); - - $type_local = PhabricatorRepositoryTransaction::TYPE_LOCAL_PATH; - - $xactions[] = id(clone $template) - ->setTransactionType($type_local) - ->setNewValue($v_local); - - try { - id(new PhabricatorRepositoryEditor()) - ->setContinueOnNoEffect(true) - ->setContentSourceFromRequest($request) - ->setActor($user) - ->applyTransactions($repository, $xactions); - - return id(new AphrontRedirectResponse())->setURI($edit_uri); - } catch (Exception $ex) { - $errors[] = $ex->getMessage(); - } - } - } - $crumbs = $this->buildApplicationCrumbs(); $crumbs->addCrumb( id(new PhabricatorCrumbView()) @@ -79,18 +46,19 @@ final class DiffusionRepositoryEditLocalController ->setUser($user) ->appendRemarkupInstructions( pht( - 'You can adjust the local path for this repository here. This is '. - 'an advanced setting and you usually should not change it.')) + "You can not adjust the local path for this repository from the ". + "web interface. To edit it, run this command:\n\n". + " phabricator/ $ ./bin/repository edit %s --as %s --local-path ...", + $repository->getCallsign(), + $user->getUsername())) ->appendChild( - id(new AphrontFormTextControl()) + id(new AphrontFormMarkupControl()) ->setName('local') ->setLabel(pht('Local Path')) - ->setValue($v_local) - ->setError($e_local)) + ->setValue($v_local)) ->appendChild( id(new AphrontFormSubmitControl()) - ->setValue(pht('Save Local')) - ->addCancelButton($edit_uri)); + ->addCancelButton($edit_uri, pht('Done'))); $object_box = id(new PHUIObjectBoxView()) ->setHeaderText($title)