1
0
Fork 0
mirror of https://we.phorge.it/source/phorge.git synced 2024-11-10 08:52:39 +01:00

Add a "touched paths" limit to repositories, limiting the maximum number of paths any commit may touch

Summary:
Depends on D19831. Ref T13216. See PHI908. Allegedly, a user copied a large repository into itself and then pushed it. Great backup strategy, but it can create headaches for administrators.

Allow a "maximum paths you can touch with one commit" limit to be configured, to make it harder for users to make this push this kind of commit by accident.

If you actually intended to do this, you can work around this by breaking your commit into pieces (or temporarily removing the limit). This isn't a security/policy sort of option, it's just a guard against silly mistakes.

Test Plan: Set limit to 2, tried to push 3 files, got rejected. Raised limit, pushed changes successfully.

Reviewers: amckinley

Reviewed By: amckinley

Maniphest Tasks: T13216

Differential Revision: https://secure.phabricator.com/D19839
This commit is contained in:
epriestley 2018-11-26 05:11:45 -08:00
parent c86c5749ba
commit 9bfe558587
8 changed files with 190 additions and 2 deletions

View file

@ -4203,6 +4203,7 @@ phutil_register_library_map(array(
'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 @@ phutil_register_library_map(array(
'PhabricatorRepositorySyncEventPHIDType' => 'PhabricatorPHIDType',
'PhabricatorRepositorySyncEventQuery' => 'PhabricatorCursorPagedPolicyAwareQuery',
'PhabricatorRepositoryTestCase' => 'PhabricatorTestCase',
'PhabricatorRepositoryTouchLimitTransaction' => 'PhabricatorRepositoryTransactionType',
'PhabricatorRepositoryTrackOnlyTransaction' => 'PhabricatorRepositoryTransactionType',
'PhabricatorRepositoryTransaction' => 'PhabricatorModularTransaction',
'PhabricatorRepositoryTransactionQuery' => 'PhabricatorApplicationTransactionQuery',

View file

@ -491,6 +491,15 @@ final class DiffusionRepositoryEditEngine
->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()),
);
}

View file

@ -39,6 +39,7 @@ final class DiffusionCommitHookEngine extends Phobject {
private $emailPHIDs = array();
private $changesets = array();
private $changesetsSize = 0;
private $filesizeCache = array();
/* -( Config )------------------------------------------------------------- */
@ -174,6 +175,15 @@ final class DiffusionCommitHookEngine extends Phobject {
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 @@ final class DiffusionCommitHookEngine extends Phobject {
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 @@ final class DiffusionCommitHookEngine extends Phobject {
}
}
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())

View file

@ -38,6 +38,7 @@ final class DiffusionRepositoryLimitsManagementPanel
return array(
'filesizeLimit',
'copyTimeLimit',
'touchLimit',
);
}
@ -95,6 +96,16 @@ final class DiffusionRepositoryLimitsManagementPanel
$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);
}

View file

@ -1926,6 +1926,14 @@ final class PhabricatorRepository extends PhabricatorRepositoryDAO
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.
*

View file

@ -25,6 +25,7 @@ final class PhabricatorRepositoryPushLog
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 @@ final class PhabricatorRepositoryPushLog
const REJECT_BROKEN = 4;
const REJECT_ENORMOUS = 5;
const REJECT_OVERSIZED = 6;
const REJECT_TOUCHES = 7;
protected $repositoryPHID;
protected $epoch;
@ -66,6 +68,7 @@ final class PhabricatorRepositoryPushLog
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 @@ final class PhabricatorRepositoryPushLog
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'),
);
}

View file

@ -0,0 +1,76 @@
<?php
final class PhabricatorRepositoryTouchLimitTransaction
extends PhabricatorRepositoryTransactionType {
const TRANSACTIONTYPE = 'limit.touch';
public function generateOldValue($object) {
return $object->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;
}
}

View file

@ -246,11 +246,38 @@ repository. If a commit creates a larger file (or modifies an existing file so
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
========