Index: src/applications/diffusion/controller/DiffusionPushLogListController.php =================================================================== --- src/applications/diffusion/controller/DiffusionPushLogListController.php +++ src/applications/diffusion/controller/DiffusionPushLogListController.php @@ -83,6 +83,10 @@ 'href' => '/r'.$callsign.$log->getRefNew(), ), $log->getRefNewShort()), + + // TODO: Make these human-readable. + $log->getChangeFlags(), + $log->getRejectCode(), phabricator_datetime($log->getEpoch(), $viewer), ); } @@ -98,6 +102,8 @@ pht('Name'), pht('Old'), pht('New'), + pht('Flags'), + pht('Code'), pht('Date'), )) ->setColumnClasses( Index: src/applications/diffusion/engine/DiffusionCommitHookEngine.php =================================================================== --- src/applications/diffusion/engine/DiffusionCommitHookEngine.php +++ src/applications/diffusion/engine/DiffusionCommitHookEngine.php @@ -1,9 +1,12 @@ remoteProtocol = $remote_protocol; return $this; @@ -88,171 +97,190 @@ return $this->viewer; } + +/* -( Hook Execution )----------------------------------------------------- */ + + public function execute() { + $ref_updates = $this->findRefUpdates(); + $all_updates = $ref_updates; + + $caught = null; + try { + + try { + $this->rejectDangerousChanges($ref_updates); + } catch (DiffusionCommitHookRejectException $ex) { + // If we're rejecting dangerous changes, flag everything that we've + // seen as rejected so it's clear that none of it was accepted. + foreach ($all_updates as $update) { + $update->setRejectCode( + PhabricatorRepositoryPushLog::REJECT_DANGEROUS); + } + throw $ex; + } + + // TODO: Fire ref herald rules. + + $content_updates = $this->findContentUpdates($ref_updates); + $all_updates = array_merge($all_updates, $content_updates); + + // TODO: Fire content Herald rules. + // TODO: Fire external hooks. + + // If we make it this far, we're accepting these changes. Mark all the + // logs as accepted. + foreach ($all_updates as $update) { + $update->setRejectCode(PhabricatorRepositoryPushLog::REJECT_ACCEPT); + } + } catch (Exception $ex) { + // We'll throw this again in a minute, but we want to save all the logs + // first. + $caught = $ex; + } + + // Save all the logs no matter what the outcome was. + foreach ($all_updates as $update) { + $update->save(); + } + + if ($caught) { + throw $caught; + } + + return 0; + } + + private function findRefUpdates() { $type = $this->getRepository()->getVersionControlSystem(); switch ($type) { case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: - $err = $this->executeGitHook(); - break; - case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: - $err = $this->executeSubversionHook(); - break; + return $this->findGitRefUpdates(); case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: - $err = $this->executeMercurialHook(); - break; + return $this->findMercurialRefUpdates(); + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + return $this->findSubversionRefUpdates(); default: throw new Exception(pht('Unsupported repository type "%s"!', $type)); } - - return $err; } - private function newPushLog() { - return PhabricatorRepositoryPushLog::initializeNewLog($this->getViewer()) - ->setRepositoryPHID($this->getRepository()->getPHID()) - ->setEpoch(time()) - ->setRemoteAddress($this->getRemoteAddressForLog()) - ->setRemoteProtocol($this->getRemoteProtocol()) - ->setTransactionKey($this->getTransactionKey()) - ->setRejectCode(PhabricatorRepositoryPushLog::REJECT_ACCEPT) - ->setRejectDetails(null); - } - - - /** - * @task git - */ - private function executeGitHook() { - $updates = $this->parseGitUpdates($this->getStdin()); + private function rejectDangerousChanges(array $ref_updates) { + assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog'); - $this->rejectGitDangerousChanges($updates); + $repository = $this->getRepository(); + if ($repository->shouldAllowDangerousChanges()) { + return; + } - // 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. + $flag_dangerous = PhabricatorRepositoryPushLog::CHANGEFLAG_DANGEROUS; - $updates = $this->findGitNewCommits($updates); + foreach ($ref_updates as $ref_update) { + if (!$ref_update->hasChangeFlags($flag_dangerous)) { + // This is not a dangerous change. + continue; + } - // TODO: Now, do content checks. + // We either have a branch deletion or a non fast-forward branch update. + // Format a message and reject the push. - // TODO: Generalize this; just getting some data in the database for now. + $message = pht( + "DANGEROUS CHANGE: %s\n". + "Dangerous change protection is enabled for this repository.\n". + "Edit the repository configuration before making dangerous changes.", + $ref_update->getDangerousChangeDescription()); - $logs = array(); - foreach ($updates as $update) { - $log = $this->newPushLog() - ->setRefType($update['type']) - ->setRefNameHash(PhabricatorHash::digestForIndex($update['ref'])) - ->setRefNameRaw($update['ref']) - ->setRefNameEncoding(phutil_is_utf8($update['ref']) ? 'utf8' : null) - ->setRefOld($update['old']) - ->setRefNew($update['new']) - ->setMergeBase(idx($update, 'merge-base')); + throw new DiffusionCommitHookRejectException($message); + } + } - $flags = 0; - if ($update['operation'] == 'create') { - $flags = $flags | PhabricatorRepositoryPushLog::CHANGEFLAG_ADD; - } else if ($update['operation'] == 'delete') { - $flags = $flags | PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE; - } else { - // TODO: This isn't correct; these might be APPEND or REWRITE, and - // if they're REWRITE they might be DANGEROUS. Fix this when this - // gets generalized. - $flags = $flags | PhabricatorRepositoryPushLog::CHANGEFLAG_APPEND; - } + private function findContentUpdates(array $ref_updates) { + assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog'); - $log->setChangeFlags($flags); - $logs[] = $log; + $type = $this->getRepository()->getVersionControlSystem(); + switch ($type) { + case PhabricatorRepositoryType::REPOSITORY_TYPE_GIT: + return $this->findGitContentUpdates($ref_updates); + case PhabricatorRepositoryType::REPOSITORY_TYPE_MERCURIAL: + return $this->findMercurialContentUpdates($ref_updates); + case PhabricatorRepositoryType::REPOSITORY_TYPE_SVN: + return $this->findSubversionContentUpdates($ref_updates); + default: + throw new Exception(pht('Unsupported repository type "%s"!', $type)); } + } - // Now, build logs for all the commits. - // TODO: Generalize this, too. - $commits = array_mergev(ipull($updates, 'commits')); - $commits = array_unique($commits); - foreach ($commits as $commit) { - $log = $this->newPushLog() - ->setRefType(PhabricatorRepositoryPushLog::REFTYPE_COMMIT) - ->setRefNew($commit) - ->setChangeFlags(PhabricatorRepositoryPushLog::CHANGEFLAG_ADD); - $logs[] = $log; - } - foreach ($logs as $log) { - $log->save(); - } +/* -( Git )---------------------------------------------------------------- */ - return 0; - } + private function findGitRefUpdates() { + $ref_updates = array(); - /** - * @task git - */ - private function parseGitUpdates($stdin) { - $updates = array(); + // First, parse stdin, which lists all the ref changes. The input looks + // like this: + // + // + $stdin = $this->getStdin(); $lines = phutil_split_lines($stdin, $retain_endings = false); foreach ($lines as $line) { $parts = explode(' ', $line, 3); if (count($parts) != 3) { throw new Exception(pht('Expected "old new ref", got "%s".', $line)); } - $update = array( - 'old' => $parts[0], - 'old.short' => substr($parts[0], 0, 8), - 'new' => $parts[1], - 'new.short' => substr($parts[1], 0, 8), - 'ref' => $parts[2], - ); - - 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/')); + + $ref_old = $parts[0]; + $ref_new = $parts[1]; + $ref_raw = $parts[2]; + + if (preg_match('(^refs/heads/)', $ref_raw)) { + $ref_type = PhabricatorRepositoryPushLog::REFTYPE_BRANCH; + } else if (preg_match('(^refs/tags/)', $ref_raw)) { + $ref_type = PhabricatorRepositoryPushLog::REFTYPE_TAG; } else { - $update['type'] = 'unknown'; + $ref_type = PhabricatorRepositoryPushLog::REFTYPE_UNKNOWN; } - $updates[] = $update; + $ref_update = $this->newPushLog() + ->setRefType($ref_type) + ->setRefName($ref_raw) + ->setRefOld($ref_old) + ->setRefNew($ref_new); + + $ref_updates[] = $ref_update; } - $updates = $this->findGitMergeBases($updates); + $this->findGitMergeBases($ref_updates); + $this->findGitChangeFlags($ref_updates); - return $updates; + return $ref_updates; } - /** - * @task git - */ - private function findGitMergeBases(array $updates) { - $empty = str_repeat('0', 40); + private function findGitMergeBases(array $ref_updates) { + assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog'); $futures = array(); - foreach ($updates as $key => $update) { - // Updates are in the form: - // - // - // + foreach ($ref_updates as $key => $ref_update) { // If the old hash is "00000...", the ref is being created (either a new // branch, or a new tag). If the new hash is "00000...", the ref is being // deleted. If both are nonempty, the ref is being updated. For updates, // we'll figure out the `merge-base` of the old and new objects here. This // lets us reject non-FF changes cheaply; later, we'll figure out exactly // which commits are new. + $ref_old = $ref_update->getRefOld(); + $ref_new = $ref_update->getRefNew(); - if ($update['old'] == $empty) { - $updates[$key]['operation'] = 'create'; - } else if ($update['new'] == $empty) { - $updates[$key]['operation'] = 'delete'; - } else { - $updates[$key]['operation'] = 'update'; - $futures[$key] = $this->getRepository()->getLocalCommandFuture( - 'merge-base %s %s', - $update['old'], - $update['new']); + if (($ref_old === self::EMPTY_HASH) || + ($ref_new === self::EMPTY_HASH)) { + continue; } + + $futures[$key] = $this->getRepository()->getLocalCommandFuture( + 'merge-base %s %s', + $ref_old, + $ref_new); } foreach (Futures($futures)->limit(8) as $key => $future) { @@ -264,20 +292,86 @@ // T4224. Assume this means there are no ancestors. list($err, $stdout) = $future->resolve(); + if ($err) { - $updates[$key]['merge-base'] = null; + $merge_base = null; } else { - $updates[$key]['merge-base'] = rtrim($stdout, "\n"); + $merge_base = rtrim($stdout, "\n"); } + + $ref_update->setMergeBase($merge_base); } - return $updates; + return $ref_updates; } - private function findGitNewCommits(array $updates) { + + private function findGitChangeFlags(array $ref_updates) { + assert_instances_of($ref_updates, 'PhabricatorRepositoryPushLog'); + + foreach ($ref_updates as $key => $ref_update) { + $ref_old = $ref_update->getRefOld(); + $ref_new = $ref_update->getRefNew(); + $ref_type = $ref_update->getRefType(); + + $ref_flags = 0; + $dangerous = null; + + if ($ref_old === self::EMPTY_HASH) { + $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_ADD; + } else if ($ref_new === self::EMPTY_HASH) { + $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE; + if ($ref_type == PhabricatorRepositoryPushLog::REFTYPE_BRANCH) { + $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_DANGEROUS; + $dangerous = pht( + "The change you're attempting to push deletes the branch '%s'.", + $ref_update->getRefName()); + } + } else { + $merge_base = $ref_update->getMergeBase(); + if ($merge_base == $ref_old) { + // This is a fast-forward update to an existing branch. + // These are safe. + $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_APPEND; + } else { + $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_REWRITE; + + // 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. Only add the dangerous + // flag if this ref is a branch. + + if ($ref_type == PhabricatorRepositoryPushLog::REFTYPE_BRANCH) { + $ref_flags |= PhabricatorRepositoryPushLog::CHANGEFLAG_DANGEROUS; + + $dangerous = 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.", + $ref_update->getRefName(), + $ref_update->getRefOldShort(), + $ref_update->getRefNewShort()); + } + } + } + + $ref_update->setChangeFlags($ref_flags); + if ($dangerous !== null) { + $ref_update->attachDangerousChangeDescription($dangerous); + } + } + + return $ref_updates; + } + + + private function findGitContentUpdates(array $ref_updates) { + $flag_delete = PhabricatorRepositoryPushLog::CHANGEFLAG_DELETE; + $futures = array(); - foreach ($updates as $key => $update) { - if ($update['operation'] == 'delete') { + foreach ($ref_updates as $key => $ref_update) { + if ($ref_update->hasChangeFlags($flag_delete)) { // Deleting a branch or tag can never create any new commits. continue; } @@ -289,84 +383,78 @@ $futures[$key] = $this->getRepository()->getLocalCommandFuture( 'log --format=%s %s --not --all', '%H', - $update['new']); + $ref_update->getRefNew()); } + $content_updates = array(); foreach (Futures($futures)->limit(8) as $key => $future) { list($stdout) = $future->resolvex(); + + if (!strlen(trim($stdout))) { + // This change doesn't have any new commits. One common case of this + // is creating a new tag which points at an existing commit. + continue; + } + $commits = phutil_split_lines($stdout, $retain_newlines = false); - $updates[$key]['commits'] = $commits; + + foreach ($commits as $commit) { + $content_updates[$commit] = $this->newPushLog() + ->setRefType(PhabricatorRepositoryPushLog::REFTYPE_COMMIT) + ->setRefNew($commit) + ->setChangeFlags(PhabricatorRepositoryPushLog::CHANGEFLAG_ADD); + } } - return $updates; + return $content_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; - } +/* -( Mercurial )---------------------------------------------------------- */ - if ($update['operation'] == 'create') { - // Creating a branch is never dangerous. - continue; - } - if ($update['operation'] == 'update') { - if ($update['old'] == $update['merge-base']) { - // This is a fast-forward update to an existing branch. - // These are safe. - continue; - } - } + private function findMercurialRefUpdates() { + // TODO: Implement. + return array(); + } - // We either have a branch deletion or a non fast-forward branch update. - // Format a message and reject the push. + private function findMercurialContentUpdates(array $ref_updates) { + // TODO: Implement. + return array(); + } - 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."); +/* -( Subversion )--------------------------------------------------------- */ - $message = $message."\n".$boilerplate; - throw new DiffusionCommitHookRejectException($message); - } + private function findSubversionRefUpdates() { + // TODO: Implement. + return array(); } - private function executeSubversionHook() { + private function findSubversionContentUpdates(array $ref_updates) { + // TODO: Implement. + return array(); + } - // TODO: Do useful things here, too. - return 0; - } +/* -( Internals )---------------------------------------------------------- */ - private function executeMercurialHook() { - // TODO: Here, too, useful things should be done. + private function newPushLog() { + // NOTE: By default, we create these with REJECT_BROKEN as the reject + // code. This indicates a broken hook, and covers the case where we + // encounter some unexpected exception and consequently reject the changes. - return 0; + return PhabricatorRepositoryPushLog::initializeNewLog($this->getViewer()) + ->attachRepository($this->getRepository()) + ->setRepositoryPHID($this->getRepository()->getPHID()) + ->setEpoch(time()) + ->setRemoteAddress($this->getRemoteAddressForLog()) + ->setRemoteProtocol($this->getRemoteProtocol()) + ->setTransactionKey($this->getTransactionKey()) + ->setRejectCode(PhabricatorRepositoryPushLog::REJECT_BROKEN) + ->setRejectDetails(null); } + } Index: src/applications/repository/storage/PhabricatorRepositoryPushLog.php =================================================================== --- src/applications/repository/storage/PhabricatorRepositoryPushLog.php +++ src/applications/repository/storage/PhabricatorRepositoryPushLog.php @@ -18,6 +18,7 @@ const REFTYPE_BOOKMARK = 'bookmark'; const REFTYPE_SVN = 'svn'; const REFTYPE_COMMIT = 'commit'; + const REFTYPE_UNKNOWN = 'unknown'; const CHANGEFLAG_ADD = 1; const CHANGEFLAG_DELETE = 2; @@ -29,6 +30,7 @@ const REJECT_DANGEROUS = 1; const REJECT_HERALD = 2; const REJECT_EXTERNAL = 3; + const REJECT_BROKEN = 4; protected $repositoryPHID; protected $epoch; @@ -47,6 +49,7 @@ protected $rejectCode; protected $rejectDetails; + private $dangerousChangeDescription = self::ATTACHABLE; private $repository = self::ATTACHABLE; public static function initializeNewLog(PhabricatorUser $viewer) { @@ -76,6 +79,16 @@ return phutil_utf8ize($this->getRefNameRaw()); } + public function setRefName($ref_raw) { + $encoding = phutil_is_utf8($ref_raw) ? 'utf8' : null; + + $this->setRefNameRaw($ref_raw); + $this->setRefNameHash(PhabricatorHash::digestForIndex($ref_raw)); + $this->setRefNameEncoding($encoding); + + return $this; + } + public function getRefOldShort() { if ($this->getRepository()->isSVN()) { return $this->getRefOld(); @@ -90,6 +103,19 @@ return substr($this->getRefNew(), 0, 12); } + public function hasChangeFlags($mask) { + return ($this->changeFlags & $mask); + } + + public function attachDangerousChangeDescription($description) { + $this->dangerousChangeDescription = $description; + return $this; + } + + public function getDangerousChangeDescription() { + return $this->assertAttached($this->dangerousChangeDescription); + } + /* -( PhabricatorPolicyInterface )----------------------------------------- */