Page MenuHomePhabricator

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

Authored by epriestley on Nov 26 2018, 1:43 PM.
Tags
None
Referenced Files
Unknown Object (File)
Tue, Nov 19, 10:23 PM
Unknown Object (File)
Sat, Nov 16, 9:23 AM
Unknown Object (File)
Tue, Nov 12, 3:10 AM
Unknown Object (File)
Tue, Nov 12, 1:45 AM
Unknown Object (File)
Fri, Nov 8, 12:52 AM
Unknown Object (File)
Mon, Nov 4, 5:14 PM
Unknown Object (File)
Mon, Nov 4, 2:28 PM
Unknown Object (File)
Mon, Nov 4, 12:55 AM
Subscribers
None

Details

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.

Diff Detail

Repository
rP Phabricator
Branch
rconfig7
Lint
Lint Passed
Unit
Tests Passed
Build Status
Buildable 21216
Build 28857: Run Core Tests
Build 28856: arc lint + arc unit

Event Timeline

  • Separate the "touch" and "filesize" code a bit better and give "Touches Too Many Paths" its own reject code.
amckinley added inline comments.
src/applications/diffusion/engine/DiffusionCommitHookEngine.php
1340

Oh this is neat. Never would have imagined this code would get reused! I guess it's not worth fiddling with this to skip the work of computing the file sizes since the vast majority of commits are reasonably-sized anyway.

1343–1349

Consistency between "affects" and "touches".

1361

Oh, and we're caching the result so we only do this work once anyway.

src/applications/diffusion/management/DiffusionRepositoryLimitsManagementPanel.php
41

touchedPathsLimit?

This revision is now accepted and ready to land.Nov 28 2018, 6:40 PM
This revision was automatically updated to reflect the committed changes.