Page Menu
Home
Phabricator
Search
Configure Global Search
Log In
Files
F15457322
D16174.diff
No One
Temporary
Actions
View File
Edit File
Delete File
View Transforms
Subscribe
Mute Notifications
Award Token
Flag For Later
Size
7 KB
Referenced Files
None
Subscribers
None
D16174.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
@@ -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
Details
Attached
Mime Type
text/plain
Expires
Mon, Mar 31, 3:39 PM (1 w, 3 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
7708177
Default Alt Text
D16174.diff (7 KB)
Attached To
Mode
D16174: Provide basic support for Subversion revprops
Attached
Detach File
Event Timeline
Log In to Comment