diff --git a/scripts/repository/commit_hook.php b/scripts/repository/commit_hook.php index 25afb714e5..405c86c196 100755 --- a/scripts/repository/commit_hook.php +++ b/scripts/repository/commit_hook.php @@ -86,6 +86,43 @@ if ($repository->isHg()) { $engine->setStdin($stdin); -$err = $engine->execute(); +try { + $err = $engine->execute(); +} catch (DiffusionCommitHookRejectException $ex) { + $console = PhutilConsole::getConsole(); + + if (PhabricatorEnv::getEnvConfig('phabricator.serious-business')) { + $preamble = pht('*** PUSH REJECTED BY COMMIT HOOK ***'); + } else { + $preamble = pht(<<writeErr("%s\n\n", $preamble); + $console->writeErr("%s\n\n", $ex->getMessage()); + $err = 1; +} exit($err); diff --git a/src/__phutil_library_map__.php b/src/__phutil_library_map__.php index 627a0a2b16..b91987885a 100644 --- a/src/__phutil_library_map__.php +++ b/src/__phutil_library_map__.php @@ -475,6 +475,7 @@ phutil_register_library_map(array( 'DiffusionCommitController' => 'applications/diffusion/controller/DiffusionCommitController.php', 'DiffusionCommitEditController' => 'applications/diffusion/controller/DiffusionCommitEditController.php', 'DiffusionCommitHookEngine' => 'applications/diffusion/engine/DiffusionCommitHookEngine.php', + 'DiffusionCommitHookRejectException' => 'applications/diffusion/exception/DiffusionCommitHookRejectException.php', 'DiffusionCommitParentsQuery' => 'applications/diffusion/query/parents/DiffusionCommitParentsQuery.php', 'DiffusionCommitQuery' => 'applications/diffusion/query/DiffusionCommitQuery.php', 'DiffusionCommitTagsController' => 'applications/diffusion/controller/DiffusionCommitTagsController.php', @@ -532,6 +533,7 @@ phutil_register_library_map(array( 'DiffusionRepositoryEditBasicController' => 'applications/diffusion/controller/DiffusionRepositoryEditBasicController.php', 'DiffusionRepositoryEditBranchesController' => 'applications/diffusion/controller/DiffusionRepositoryEditBranchesController.php', 'DiffusionRepositoryEditController' => 'applications/diffusion/controller/DiffusionRepositoryEditController.php', + 'DiffusionRepositoryEditDangerousController' => 'applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php', 'DiffusionRepositoryEditDeleteController' => 'applications/diffusion/controller/DiffusionRepositoryEditDeleteController.php', 'DiffusionRepositoryEditEncodingController' => 'applications/diffusion/controller/DiffusionRepositoryEditEncodingController.php', 'DiffusionRepositoryEditHostingController' => 'applications/diffusion/controller/DiffusionRepositoryEditHostingController.php', @@ -2806,6 +2808,7 @@ phutil_register_library_map(array( 'DiffusionCommitController' => 'DiffusionController', 'DiffusionCommitEditController' => 'DiffusionController', 'DiffusionCommitHookEngine' => 'Phobject', + 'DiffusionCommitHookRejectException' => 'Exception', 'DiffusionCommitParentsQuery' => 'DiffusionQuery', 'DiffusionCommitQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'DiffusionCommitTagsController' => 'DiffusionController', @@ -2854,6 +2857,7 @@ phutil_register_library_map(array( 'DiffusionRepositoryEditBasicController' => 'DiffusionRepositoryEditController', 'DiffusionRepositoryEditBranchesController' => 'DiffusionRepositoryEditController', 'DiffusionRepositoryEditController' => 'DiffusionController', + 'DiffusionRepositoryEditDangerousController' => 'DiffusionRepositoryEditController', 'DiffusionRepositoryEditDeleteController' => 'DiffusionRepositoryEditController', 'DiffusionRepositoryEditEncodingController' => 'DiffusionRepositoryEditController', 'DiffusionRepositoryEditHostingController' => 'DiffusionRepositoryEditController', diff --git a/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php b/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php index 9ca2c6ab52..b4b0c1dd30 100644 --- a/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php +++ b/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php @@ -70,6 +70,7 @@ final class PhabricatorApplicationDiffusion extends PhabricatorApplication { 'basic/' => 'DiffusionRepositoryEditBasicController', 'encoding/' => 'DiffusionRepositoryEditEncodingController', 'activate/' => 'DiffusionRepositoryEditActivateController', + 'dangerous/' => 'DiffusionRepositoryEditDangerousController', 'policy/' => 'DiffusionRepositoryEditPolicyController', 'branches/' => 'DiffusionRepositoryEditBranchesController', 'subversion/' => 'DiffusionRepositoryEditSubversionController', diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php new file mode 100644 index 0000000000..97779a8839 --- /dev/null +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php @@ -0,0 +1,78 @@ +getRequest(); + $viewer = $request->getUser(); + $drequest = $this->diffusionRequest; + $repository = $drequest->getRepository(); + + $repository = id(new PhabricatorRepositoryQuery()) + ->setViewer($viewer) + ->requireCapabilities( + array( + PhabricatorPolicyCapability::CAN_VIEW, + PhabricatorPolicyCapability::CAN_EDIT, + )) + ->withIDs(array($repository->getID())) + ->executeOne(); + + if (!$repository) { + return new Aphront404Response(); + } + + if (!$repository->canAllowDangerousChanges()) { + return new Aphront400Response(); + } + + $edit_uri = $this->getRepositoryControllerURI($repository, 'edit/'); + + if ($request->isFormPost()) { + $xaction = id(new PhabricatorRepositoryTransaction()) + ->setTransactionType(PhabricatorRepositoryTransaction::TYPE_DANGEROUS) + ->setNewValue(!$repository->shouldAllowDangerousChanges()); + + $editor = id(new PhabricatorRepositoryEditor()) + ->setContinueOnNoEffect(true) + ->setContentSourceFromRequest($request) + ->setActor($viewer) + ->applyTransactions($repository, array($xaction)); + + return id(new AphrontReloadResponse())->setURI($edit_uri); + } + + $dialog = id(new AphrontDialogView()) + ->setUser($viewer); + + $force = phutil_tag('tt', array(), '--force'); + + if ($repository->shouldAllowDangerousChanges()) { + $dialog + ->setTitle(pht('Prevent Dangerous changes?')) + ->appendChild( + pht( + 'It will no longer be possible to delete branches from this '. + 'repository, or %s push to this repository.', + $force)) + ->addSubmitButton(pht('Prevent Dangerous Changes')) + ->addCancelButton($edit_uri); + } else { + $dialog + ->setTitle(pht('Allow Dangerous Changes?')) + ->appendChild( + pht( + 'If you allow dangerous changes, it will be possible to delete '. + 'branches and %s push this repository. These operations can '. + 'alter a repository in a way that is difficult to recover from.', + $force)) + ->addSubmitButton(pht('Allow Dangerous Changes')) + ->addCancelButton($edit_uri); + } + + return id(new AphrontDialogResponse()) + ->setDialog($dialog); + } + +} diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php index de624a259f..0b058744d6 100644 --- a/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php +++ b/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php @@ -577,6 +577,25 @@ final class DiffusionRepositoryEditMainController $this->getRepositoryControllerURI($repository, 'edit/hosting/')); $view->addAction($edit); + if ($repository->canAllowDangerousChanges()) { + if ($repository->shouldAllowDangerousChanges()) { + $changes = id(new PhabricatorActionView()) + ->setIcon('blame') + ->setName(pht('Prevent Dangerous Changes')) + ->setHref( + $this->getRepositoryControllerURI($repository, 'edit/dangerous/')) + ->setWorkflow(true); + } else { + $changes = id(new PhabricatorActionView()) + ->setIcon('warning') + ->setName(pht('Allow Dangerous Changes')) + ->setHref( + $this->getRepositoryControllerURI($repository, 'edit/dangerous/')) + ->setWorkflow(true); + } + $view->addAction($changes); + } + return $view; } @@ -611,6 +630,18 @@ final class DiffusionRepositoryEditMainController PhabricatorRepository::getProtocolAvailabilityName( $repository->getServeOverSSH()))); + if ($repository->canAllowDangerousChanges()) { + if ($repository->shouldAllowDangerousChanges()) { + $description = pht('Allowed'); + } else { + $description = pht('Not Allowed'); + } + + $view->addProperty( + pht('Dangerous Changes'), + $description); + } + return $view; } diff --git a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php index b4a5c1e5fd..c91af84c11 100644 --- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -67,6 +67,8 @@ final class DiffusionCommitHookEngine extends Phobject { private function executeGitHook() { $updates = $this->parseGitUpdates($this->getStdin()); + $this->rejectGitDangerousChanges($updates); + // TODO: Do cheap checks: non-ff commits, mutating refs without access, // creating or deleting things you can't touch. We can do all non-content // checks here. @@ -101,8 +103,10 @@ final class DiffusionCommitHookEngine extends Phobject { if (preg_match('(^refs/heads/)', $update['ref'])) { $update['type'] = 'branch'; + $update['ref.short'] = substr($update['ref'], strlen('refs/heads/')); } else if (preg_match('(^refs/tags/)', $update['ref'])) { $update['type'] = 'tag'; + $update['ref.short'] = substr($update['ref'], strlen('refs/tags/')); } else { $update['type'] = 'unknown'; } @@ -159,7 +163,7 @@ final class DiffusionCommitHookEngine extends Phobject { private function findGitNewCommits(array $updates) { $futures = array(); foreach ($updates as $key => $update) { - if ($update['type'] == 'delete') { + if ($update['operation'] == 'delete') { // Deleting a branch or tag can never create any new commits. continue; } @@ -183,6 +187,61 @@ final class DiffusionCommitHookEngine extends Phobject { return $updates; } + private function rejectGitDangerousChanges(array $updates) { + $repository = $this->getRepository(); + if ($repository->shouldAllowDangerousChanges()) { + return; + } + + foreach ($updates as $update) { + if ($update['type'] != 'branch') { + // For now, we don't consider deleting or moving tags to be a + // "dangerous" update. It's way harder to get wrong and should be easy + // to recover from once we have better logging. + continue; + } + + if ($update['operation'] == 'create') { + // Creating a branch is never dangerous. + continue; + } + + if ($update['operation'] == 'change') { + if ($update['old'] == $update['merge-base']) { + // This is a fast-forward update to an existing branch. + // These are safe. + continue; + } + } + + // We either have a branch deletion or a non fast-forward branch update. + // Format a message and reject the push. + + if ($update['operation'] == 'delete') { + $message = pht( + "DANGEROUS CHANGE: The change you're attempting to push deletes ". + "the branch '%s'.", + $update['ref.short']); + } else { + $message = pht( + "DANGEROUS CHANGE: The change you're attempting to push updates ". + "the branch '%s' from '%s' to '%s', but this is not a fast-forward. ". + "Pushes which rewrite published branch history are dangerous.", + $update['ref.short'], + $update['old.short'], + $update['new.short']); + } + + $boilerplate = pht( + "Dangerous change protection is enabled for this repository.\n". + "Edit the repository configuration before making dangerous changes."); + + $message = $message."\n".$boilerplate; + + throw new DiffusionCommitHookRejectException($message); + } + } + private function executeSubversionHook() { // TODO: Do useful things here, too. diff --git a/src/applications/diffusion/exception/DiffusionCommitHookRejectException.php b/src/applications/diffusion/exception/DiffusionCommitHookRejectException.php new file mode 100644 index 0000000000..9860bda7d9 --- /dev/null +++ b/src/applications/diffusion/exception/DiffusionCommitHookRejectException.php @@ -0,0 +1,5 @@ +getPushPolicy(); case PhabricatorRepositoryTransaction::TYPE_CREDENTIAL: return $object->getCredentialPHID(); + case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: + return $object->shouldAllowDangerousChanges(); } } @@ -110,6 +113,7 @@ final class PhabricatorRepositoryEditor case PhabricatorRepositoryTransaction::TYPE_PROTOCOL_SSH: case PhabricatorRepositoryTransaction::TYPE_PUSH_POLICY: case PhabricatorRepositoryTransaction::TYPE_CREDENTIAL: + case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: return $xaction->getNewValue(); case PhabricatorRepositoryTransaction::TYPE_NOTIFY: case PhabricatorRepositoryTransaction::TYPE_AUTOCLOSE: @@ -175,6 +179,9 @@ final class PhabricatorRepositoryEditor return $object->setPushPolicy($xaction->getNewValue()); case PhabricatorRepositoryTransaction::TYPE_CREDENTIAL: return $object->setCredentialPHID($xaction->getNewValue()); + case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: + $object->setDetail('allow-dangerous-changes', $xaction->getNewValue()); + return; case PhabricatorRepositoryTransaction::TYPE_ENCODING: // Make sure the encoding is valid by converting to UTF-8. This tests // that the user has mbstring installed, and also that they didn't type @@ -285,6 +292,7 @@ final class PhabricatorRepositoryEditor case PhabricatorRepositoryTransaction::TYPE_PROTOCOL_SSH: case PhabricatorRepositoryTransaction::TYPE_PUSH_POLICY: case PhabricatorRepositoryTransaction::TYPE_CREDENTIAL: + case PhabricatorRepositoryTransaction::TYPE_DANGEROUS: PhabricatorPolicyFilter::requireCapability( $this->requireActor(), $object, diff --git a/src/applications/repository/storage/PhabricatorRepository.php b/src/applications/repository/storage/PhabricatorRepository.php index 39328458bb..403a4a55ae 100644 --- a/src/applications/repository/storage/PhabricatorRepository.php +++ b/src/applications/repository/storage/PhabricatorRepository.php @@ -911,6 +911,22 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO return false; } + public function canAllowDangerousChanges() { + if (!$this->isHosted()) { + return false; + } + + if ($this->isGit()) { + return true; + } + + return false; + } + + public function shouldAllowDangerousChanges() { + return (bool)$this->getDetail('allow-dangerous-changes'); + } + public function writeStatusMessage( $status_type, $status_code, diff --git a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php index 84c74276ab..4ad20f8b0a 100644 --- a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php +++ b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php @@ -22,6 +22,7 @@ final class PhabricatorRepositoryTransaction const TYPE_PROTOCOL_SSH = 'repo:serve-ssh'; const TYPE_PUSH_POLICY = 'repo:push-policy'; const TYPE_CREDENTIAL = 'repo:credential'; + const TYPE_DANGEROUS = 'repo:dangerous'; // TODO: Clean up these legacy transaction types. const TYPE_SSH_LOGIN = 'repo:ssh-login'; @@ -338,6 +339,16 @@ final class PhabricatorRepositoryTransaction $this->renderHandleLink($author_phid), $this->renderPolicyName($old), $this->renderPolicyName($new)); + case self::TYPE_DANGEROUS: + if ($new) { + return pht( + '%s disabled protection against dangerous changes.', + $this->renderHandleLink($author_phid)); + } else { + return pht( + '%s enabled protection against dangerous changes.', + $this->renderHandleLink($author_phid)); + } } return parent::getTitle();