Page MenuHomePhabricator
Files F87668

D7689.diff
All Users
Actions

D7689.diff
View Options

diff --git a/scripts/repository/commit_hook.php b/scripts/repository/commit_hook.php
--- a/scripts/repository/commit_hook.php
+++ b/scripts/repository/commit_hook.php
@@ -86,6 +86,43 @@
$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(<<<EOTXT
++---------------------------------------------------------------+
+| * * * PUSH REJECTED BY EVIL DRAGON BUREAUCRATS * * * |
++---------------------------------------------------------------+
+ \
+ \ ^ /^
+ \ / \ // \
+ \ |\___/| / \// .\
+ \ /V V \__ / // | \ \ *----*
+ / / \/_/ // | \ \ \ |
+ @___@` \/_ // | \ \ \/\ \
+ 0/0/| \/_ // | \ \ \ \
+ 0/0/0/0/| \/// | \ \ | |
+ 0/0/0/0/0/_|_ / ( // | \ _\ | /
+ 0/0/0/0/0/0/`/,_ _ _/ ) ; -. | _ _\.-~ / /
+ ,-} _ *-.|.-~-. .~ ~
+ \ \__/ `/\ / ~-. _ .-~ /
+ \____(Oo) *. } { /
+ ( (--) .----~-.\ \-` .~
+ //__\\\\ \ DENIED! ///.----..< \ _ -~
+ // \\\\ ///-._ _ _ _ _ _ _{^ - - - - ~
+
+EOTXT
+);
+ }
+
+ $console->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
--- a/src/__phutil_library_map__.php
+++ b/src/__phutil_library_map__.php
@@ -475,6 +475,7 @@
'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 @@
'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 @@
'DiffusionCommitController' => 'DiffusionController',
'DiffusionCommitEditController' => 'DiffusionController',
'DiffusionCommitHookEngine' => 'Phobject',
+ 'DiffusionCommitHookRejectException' => 'Exception',
'DiffusionCommitParentsQuery' => 'DiffusionQuery',
'DiffusionCommitQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'DiffusionCommitTagsController' => 'DiffusionController',
@@ -2854,6 +2857,7 @@
'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
--- a/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php
+++ b/src/applications/diffusion/application/PhabricatorApplicationDiffusion.php
@@ -70,6 +70,7 @@
'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
--- /dev/null
+++ b/src/applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php
@@ -0,0 +1,78 @@
+<?php
+
+final class DiffusionRepositoryEditDangerousController
+ extends DiffusionRepositoryEditController {
+
+ public function processRequest() {
+ $request = $this->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
--- a/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryEditMainController.php
@@ -577,6 +577,25 @@
$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 @@
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
--- a/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
+++ b/src/applications/diffusion/engine/DiffusionCommitHookEngine.php
@@ -67,6 +67,8 @@
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 @@
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 @@
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 @@
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
--- /dev/null
+++ b/src/applications/diffusion/exception/DiffusionCommitHookRejectException.php
@@ -0,0 +1,5 @@
+<?php
+
+final class DiffusionCommitHookRejectException extends Exception {
+
+}
diff --git a/src/applications/repository/editor/PhabricatorRepositoryEditor.php b/src/applications/repository/editor/PhabricatorRepositoryEditor.php
--- a/src/applications/repository/editor/PhabricatorRepositoryEditor.php
+++ b/src/applications/repository/editor/PhabricatorRepositoryEditor.php
@@ -30,6 +30,7 @@
$types[] = PhabricatorRepositoryTransaction::TYPE_PROTOCOL_SSH;
$types[] = PhabricatorRepositoryTransaction::TYPE_PUSH_POLICY;
$types[] = PhabricatorRepositoryTransaction::TYPE_CREDENTIAL;
+ $types[] = PhabricatorRepositoryTransaction::TYPE_DANGEROUS;
$types[] = PhabricatorTransactions::TYPE_VIEW_POLICY;
$types[] = PhabricatorTransactions::TYPE_EDIT_POLICY;
@@ -80,6 +81,8 @@
return $object->getPushPolicy();
case PhabricatorRepositoryTransaction::TYPE_CREDENTIAL:
return $object->getCredentialPHID();
+ case PhabricatorRepositoryTransaction::TYPE_DANGEROUS:
+ return $object->shouldAllowDangerousChanges();
}
}
@@ -110,6 +113,7 @@
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 @@
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 @@
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
--- a/src/applications/repository/storage/PhabricatorRepository.php
+++ b/src/applications/repository/storage/PhabricatorRepository.php
@@ -911,6 +911,22 @@
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
--- a/src/applications/repository/storage/PhabricatorRepositoryTransaction.php
+++ b/src/applications/repository/storage/PhabricatorRepositoryTransaction.php
@@ -22,6 +22,7 @@
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 @@
$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();

File Metadata

Mime Type
text/x-diff
Storage Engine
amazon-s3
Storage Format
Raw Data
Storage Handle
phabricator/w3/av/bnrzjfdonz3fw3lk
Default Alt Text
D7689.diff (17 KB)

Event Timeline

Phabricator · Built by Phacility