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 @@ -4203,6 +4203,7 @@ 'PhabricatorRepositorySyncEventPHIDType' => 'applications/repository/phid/PhabricatorRepositorySyncEventPHIDType.php', 'PhabricatorRepositorySyncEventQuery' => 'applications/repository/query/PhabricatorRepositorySyncEventQuery.php', 'PhabricatorRepositoryTestCase' => 'applications/repository/storage/__tests__/PhabricatorRepositoryTestCase.php', + 'PhabricatorRepositoryTouchLimitTransaction' => 'applications/repository/xaction/PhabricatorRepositoryTouchLimitTransaction.php', 'PhabricatorRepositoryTrackOnlyTransaction' => 'applications/repository/xaction/PhabricatorRepositoryTrackOnlyTransaction.php', 'PhabricatorRepositoryTransaction' => 'applications/repository/storage/PhabricatorRepositoryTransaction.php', 'PhabricatorRepositoryTransactionQuery' => 'applications/repository/query/PhabricatorRepositoryTransactionQuery.php', @@ -10198,6 +10199,7 @@ 'PhabricatorRepositorySyncEventPHIDType' => 'PhabricatorPHIDType', 'PhabricatorRepositorySyncEventQuery' => 'PhabricatorCursorPagedPolicyAwareQuery', 'PhabricatorRepositoryTestCase' => 'PhabricatorTestCase', + 'PhabricatorRepositoryTouchLimitTransaction' => 'PhabricatorRepositoryTransactionType', 'PhabricatorRepositoryTrackOnlyTransaction' => 'PhabricatorRepositoryTransactionType', 'PhabricatorRepositoryTransaction' => 'PhabricatorModularTransaction', 'PhabricatorRepositoryTransactionQuery' => 'PhabricatorApplicationTransactionQuery', diff --git a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php --- a/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php +++ b/src/applications/diffusion/editor/DiffusionRepositoryEditEngine.php @@ -491,6 +491,15 @@ ->setConduitDescription(pht('Change the copy time limit.')) ->setConduitTypeDescription(pht('New repository copy time limit.')) ->setValue($object->getCopyTimeLimit()), + id(new PhabricatorTextEditField()) + ->setKey('touchLimit') + ->setLabel(pht('Touched Paths Limit')) + ->setTransactionType( + PhabricatorRepositoryTouchLimitTransaction::TRANSACTIONTYPE) + ->setDescription(pht('Maximum permitted paths touched per commit.')) + ->setConduitDescription(pht('Change the touch limit.')) + ->setConduitTypeDescription(pht('New repository touch limit.')) + ->setValue($object->getTouchLimit()), ); } 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 @@ -39,6 +39,7 @@ private $emailPHIDs = array(); private $changesets = array(); private $changesetsSize = 0; + private $filesizeCache = array(); /* -( Config )------------------------------------------------------------- */ @@ -174,6 +175,15 @@ throw $ex; } + try { + if (!$is_initial_import) { + $this->rejectCommitsAffectingTooManyPaths($content_updates); + } + } catch (DiffusionCommitHookRejectException $ex) { + $this->rejectCode = PhabricatorRepositoryPushLog::REJECT_TOUCHES; + throw $ex; + } + try { if (!$is_initial_import) { $this->rejectEnormousChanges($content_updates); @@ -1276,7 +1286,8 @@ foreach ($content_updates as $update) { $identifier = $update->getRefNew(); - $sizes = $this->loadFileSizesForCommit($identifier); + $sizes = $this->getFileSizesForCommit($identifier); + foreach ($sizes as $path => $size) { if ($size <= $limit) { continue; @@ -1301,7 +1312,47 @@ } } - public function loadFileSizesForCommit($identifier) { + private function rejectCommitsAffectingTooManyPaths(array $content_updates) { + $repository = $this->getRepository(); + + $limit = $repository->getTouchLimit(); + if (!$limit) { + return; + } + + foreach ($content_updates as $update) { + $identifier = $update->getRefNew(); + + $sizes = $this->getFileSizesForCommit($identifier); + if (count($sizes) > $limit) { + $message = pht( + 'COMMIT AFFECTS TOO MANY PATHS'. + "\n". + 'This repository ("%s") is configured with a touched files limit '. + 'that caps the maximum number of paths any single commit may '. + 'affect. You are pushing a change ("%s") which exceeds this '. + 'limit: it affects %s paths, but the largest number of paths any '. + 'commit may affect is %s paths.', + $repository->getDisplayName(), + $identifier, + phutil_count($sizes), + new PhutilNumber($limit)); + + throw new DiffusionCommitHookRejectException($message); + } + } + } + + public function getFileSizesForCommit($identifier) { + if (!isset($this->filesizeCache[$identifier])) { + $file_sizes = $this->loadFileSizesForCommit($identifier); + $this->filesizeCache[$identifier] = $file_sizes; + } + + return $this->filesizeCache[$identifier]; + } + + private function loadFileSizesForCommit($identifier) { $repository = $this->getRepository(); return id(new DiffusionLowLevelFilesizeQuery()) diff --git a/src/applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php b/src/applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php --- a/src/applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php +++ b/src/applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php @@ -38,6 +38,7 @@ return array( 'filesizeLimit', 'copyTimeLimit', + 'touchLimit', ); } @@ -95,6 +96,16 @@ $view->addProperty(pht('Clone/Fetch Timeout'), $copy_display); + $touch_limit = $repository->getTouchLimit(); + if ($touch_limit) { + $touch_display = pht('%s Paths', new PhutilNumber($touch_limit)); + } else { + $touch_display = pht('Unlimited'); + $touch_display = phutil_tag('em', array(), $touch_display); + } + + $view->addProperty(pht('Touched Paths Limit'), $touch_display); + return $this->newBox(pht('Limits'), $view); } 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 @@ -1926,6 +1926,14 @@ return $this->setDetail('limit.filesize', $limit); } + public function getTouchLimit() { + return $this->getDetail('limit.touch'); + } + + public function setTouchLimit($limit) { + return $this->setDetail('limit.touch', $limit); + } + /** * Retrieve the service URI for the device hosting this repository. * diff --git a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php --- a/src/applications/repository/storage/PhabricatorRepositoryPushLog.php +++ b/src/applications/repository/storage/PhabricatorRepositoryPushLog.php @@ -25,6 +25,7 @@ const CHANGEFLAG_DANGEROUS = 16; const CHANGEFLAG_ENORMOUS = 32; const CHANGEFLAG_OVERSIZED = 64; + const CHANGEFLAG_TOUCHES = 128; const REJECT_ACCEPT = 0; const REJECT_DANGEROUS = 1; @@ -33,6 +34,7 @@ const REJECT_BROKEN = 4; const REJECT_ENORMOUS = 5; const REJECT_OVERSIZED = 6; + const REJECT_TOUCHES = 7; protected $repositoryPHID; protected $epoch; @@ -66,6 +68,7 @@ self::CHANGEFLAG_DANGEROUS => pht('Dangerous'), self::CHANGEFLAG_ENORMOUS => pht('Enormous'), self::CHANGEFLAG_OVERSIZED => pht('Oversized'), + self::CHANGEFLAG_TOUCHES => pht('Touches Too Many Paths'), ); } @@ -78,6 +81,7 @@ self::REJECT_BROKEN => pht('Rejected: Broken'), self::REJECT_ENORMOUS => pht('Rejected: Enormous'), self::REJECT_OVERSIZED => pht('Rejected: Oversized File'), + self::REJECT_TOUCHES => pht('Rejected: Touches Too Many Paths'), ); } diff --git a/src/applications/repository/xaction/PhabricatorRepositoryTouchLimitTransaction.php b/src/applications/repository/xaction/PhabricatorRepositoryTouchLimitTransaction.php new file mode 100644 --- /dev/null +++ b/src/applications/repository/xaction/PhabricatorRepositoryTouchLimitTransaction.php @@ -0,0 +1,76 @@ +getTouchLimit(); + } + + public function generateNewValue($object, $value) { + if (!strlen($value)) { + return null; + } + + $value = (int)$value; + if (!$value) { + return null; + } + + return $value; + } + + public function applyInternalEffects($object, $value) { + $object->setTouchLimit($value); + } + + public function getTitle() { + $old = $this->getOldValue(); + $new = $this->getNewValue(); + + if ($old && $new) { + return pht( + '%s changed the touch limit for this repository from %s paths to '. + '%s paths.', + $this->renderAuthor(), + $this->renderOldValue(), + $this->renderNewValue()); + } else if ($new) { + return pht( + '%s set the touch limit for this repository to %s paths.', + $this->renderAuthor(), + $this->renderNewValue()); + } else { + return pht( + '%s removed the touch limit (%s paths) for this repository.', + $this->renderAuthor(), + $this->renderOldValue()); + } + } + + public function validateTransactions($object, array $xactions) { + $errors = array(); + + foreach ($xactions as $xaction) { + $new = $xaction->getNewValue(); + + if (!strlen($new)) { + continue; + } + + if (!preg_match('/^\d+\z/', $new)) { + $errors[] = $this->newInvalidError( + pht( + 'Unable to parse touch limit, specify a positive number of '. + 'paths.'), + $xaction); + continue; + } + } + + return $errors; + } + +} diff --git a/src/docs/user/userguide/diffusion_managing.diviner b/src/docs/user/userguide/diffusion_managing.diviner --- a/src/docs/user/userguide/diffusion_managing.diviner +++ b/src/docs/user/userguide/diffusion_managing.diviner @@ -246,11 +246,38 @@ it becomes too large) it will be rejected. This option only applies to hosted repositories. +This limit is primarily intended to make it more difficult to accidentally push +very large files that shouldn't be version controlled (like logs, binaries, +machine learning data, or media assets). Pushing huge datafiles by mistake can +make the repository unwieldy by dramatically increasing how much data must be +transferred over the network to clone it, and simply reverting the changes +doesn't reduce the impact of this kind of mistake. + **Clone/Fetch Timeout**: Configure the internal timeout for creating copies of this repository during operations like intracluster synchronization and Drydock working copy construction. This timeout does not affect external users. +**Touch Limit**: Apply a limit to the maximum number of paths that any commit +may touch. If a commit affects more paths than this limit, it will be rejected. +This option only applies to hosted repositories. Users may work around this +limit by breaking the commit into several smaller commits which each affect +fewer paths. + +This limit is intended to offer a guard rail against users making silly +mistakes that create obviously mistaken changes, like copying an entire +repository into itself and pushing the result. This kind of change can take +some effort to clean up if it becomes part of repository history. + +Note that if you move a file, both the old and new locations count as touched +paths. You should generally configure this limit to be more than twice the +number of files you anticipate any user ever legitimately wanting to move in +a single commit. For example, a limit of `20000` will let users move up to +10,000 files in a single commit, but will reject users mistakenly trying to +push a copy of another repository or a directory with a million logfiles or +whatever other kind of creative nonsense they manage to dream up. + + Branches ========