Page MenuHomePhabricator

D16174.id38908.diff
No OneTemporary

D16174.id38908.diff

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
@@ -54,10 +54,62 @@
$engine->setRepository($repository);
+$args = new PhutilArgumentParser($argv);
+$args->parsePartial(
+ array(
+ array(
+ 'name' => 'hook-mode',
+ 'param' => 'mode',
+ 'help' => pht('Hook execution mode.'),
+ ),
+ ));
+
+$argv = array_merge(
+ array($argv[0]),
+ $args->getUnconsumedArgumentVector());
// Figure out which user is writing the commit.
+$hook_mode = $args->getArg('hook-mode');
+if ($hook_mode !== null) {
+ $known_modes = array(
+ 'svn-revprop' => true,
+ );
-if ($repository->isGit() || $repository->isHg()) {
+ if (empty($known_modes[$hook_mode])) {
+ throw new Exception(
+ pht(
+ 'Invalid Hook Mode: This hook was invoked in "%s" mode, but this '.
+ 'is not a recognized hook mode. Valid modes are: %s.',
+ $hook_mode,
+ implode(', ', array_keys($known_modes))));
+ }
+}
+
+$is_svnrevprop = ($hook_mode == 'svn-revprop');
+
+if ($is_svnrevprop) {
+ // For now, we let these through if the repository allows dangerous changes
+ // and prevent them if it doesn't. See T11208 for discussion.
+
+ $revprop_key = $argv[5];
+
+ if ($repository->shouldAllowDangerousChanges()) {
+ $err = 0;
+ } else {
+ $err = 1;
+
+ $console = PhutilConsole::getConsole();
+ $console->writeErr(
+ pht(
+ "DANGEROUS CHANGE: Dangerous change protection is enabled for this ".
+ "repository, so you can not change revision properties (you are ".
+ "attempting to edit \"%s\").\n".
+ "Edit the repository configuration before making dangerous changes.",
+ $revprop_key));
+ }
+
+ exit($err);
+} else if ($repository->isGit() || $repository->isHg()) {
$username = getenv(DiffusionCommitHookEngine::ENV_USER);
if (!strlen($username)) {
throw new Exception(
diff --git a/src/applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php b/src/applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php
--- a/src/applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php
+++ b/src/applications/diffusion/controller/DiffusionRepositoryEditDangerousController.php
@@ -18,25 +18,14 @@
->getPanelURI();
if (!$repository->canAllowDangerousChanges()) {
- if ($repository->isSVN()) {
- return $this->newDialog()
- ->setTitle(pht('Not in Danger'))
- ->appendParagraph(
- pht(
- 'It is not possible for users to push any dangerous changes '.
- 'to a Subversion repository. Pushes to a Subversion repository '.
- 'can always be reverted and never destroy data.'))
- ->addCancelButton($panel_uri);
- } else {
- return $this->newDialog()
- ->setTitle(pht('Unprotectable Repository'))
- ->appendParagraph(
- pht(
- 'This repository can not be protected from dangerous changes '.
- 'because Phabricator does not control what users are allowed '.
- 'to push to it.'))
- ->addCancelButton($panel_uri);
- }
+ return $this->newDialog()
+ ->setTitle(pht('Unprotectable Repository'))
+ ->appendParagraph(
+ pht(
+ 'This repository can not be protected from dangerous changes '.
+ 'because Phabricator does not control what users are allowed '.
+ 'to push to it.'))
+ ->addCancelButton($panel_uri);
}
if ($request->isFormPost()) {
@@ -57,18 +46,34 @@
if ($repository->shouldAllowDangerousChanges()) {
$title = pht('Prevent Dangerous Changes');
- $body = pht(
- 'It will no longer be possible to delete branches from this '.
- 'repository, or %s push to this repository.',
- $force);
+
+ if ($repository->isSVN()) {
+ $body = pht(
+ 'It will no longer be possible to edit revprops in this '.
+ 'repository.');
+ } else {
+ $body = pht(
+ 'It will no longer be possible to delete branches from this '.
+ 'repository, or %s push to this repository.',
+ $force);
+ }
+
$submit = pht('Prevent Dangerous Changes');
} else {
$title = pht('Allow Dangerous Changes');
- $body = 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);
+ if ($repository->isSVN()) {
+ $body = pht(
+ 'If you allow dangerous changes, it will be possible to edit '.
+ 'reprops in this repository, including arbitrarily rewriting '.
+ 'commit messages. These operations can alter a repository in a '.
+ 'way that is difficult to recover from.');
+ } else {
+ $body = 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);
+ }
$submit = pht('Allow Dangerous Changes');
}
diff --git a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
--- a/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
+++ b/src/applications/repository/engine/PhabricatorRepositoryPullEngine.php
@@ -191,7 +191,7 @@
));
}
- private function installHook($path) {
+ private function installHook($path, array $hook_argv = array()) {
$this->log('%s', pht('Installing commit hook to "%s"...', $path));
$repository = $this->getRepository();
@@ -202,10 +202,11 @@
$full_php_path = Filesystem::resolveBinary('php');
$cmd = csprintf(
- 'exec %s -f %s -- %s "$@"',
+ 'exec %s -f %s -- %s %Ls "$@"',
$full_php_path,
$bin,
- $identifier);
+ $identifier,
+ $hook_argv);
$hook = "#!/bin/sh\nexport TERM=dumb\n{$cmd}\n";
@@ -585,8 +586,16 @@
$root = $repository->getLocalPath();
$path = '/hooks/pre-commit';
-
$this->installHook($root.$path);
+
+ $revprop_path = '/hooks/pre-revprop-change';
+
+ $revprop_argv = array(
+ '--hook-mode',
+ 'svn-revprop',
+ );
+
+ $this->installHook($root.$revprop_path, $revprop_argv);
}
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
@@ -1623,11 +1623,10 @@
return false;
}
- if ($this->isGit() || $this->isHg()) {
- return true;
- }
+ // In Git and Mercurial, ref deletions and rewrites are dangerous.
+ // In Subversion, editing revprops is dangerous.
- return false;
+ return true;
}
public function shouldAllowDangerousChanges() {

File Metadata

Mime Type
text/plain
Expires
Fri, Sep 20, 10:18 AM (14 h, 52 m)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6623336
Default Alt Text
D16174.id38908.diff (7 KB)

Event Timeline