Page MenuHomePhabricator

Prevent "enormous" commits from being pushed to hosted repositories in Diffusion by default
Closed, ResolvedPublic

Description

See T8644#233754.

Herald can not execute content rules on sufficiently large changes. A practical limit here is when a change has so much text that it can not fit in memory, we can not run regular expressions on that text. The current effective limit is 1GB of text, or more than 15 minutes executing git diff (or similar) to pull the change out of the VCS.

Thus, you can bypass Herald content checks by making a forbidden change alongside an enormous change. For example, if a repository forbids the word "tortise" from appearing in the source using a content-testing Herald rule, you can add the word "tortise" like this:

  • Add the word "tortise" to some file.
  • Add a 1.5GB file of any text.
  • Commit and push.

The 1.5GB file will effectively disable the content testing rule. Because the recently added LFS rule (D18827) is fundamentally a content rule, it can also be bypassed in this way.

Today, protecting against this requires writing a second rule which blocks all enormous changes:

When:
[ Diff is enormous ] [ is true ]
Take actions:
[ Block push with message ] [ "..." ]

This rule is implemented here as H68 No Enormous Commits.

This makes things safe, but is not obvious and should not need to be written. One way to fix the defaults is:

  • Block these changes by default.
  • Add a new "Allow Enormous Changes" option at the repository level, similar to "Allow Dangerous Changes", that defaults off.

This would give us safe defaults, and make "Enormous" and "Dangerous" changes behave similarly (you can turn off the global blocks and filter them selectively if you have a need for them).

Event Timeline

epriestley triaged this task as Normal priority.Dec 18 2017, 4:45 PM
epriestley created this task.

Adjacently, some other parts of the UI currently use the word "enormous" (see https://discourse.phabricator-community.org/t/herald-enormous-check/822). They should be rewritten to use different language (like "Very Large"). We should use the term "Enormous" to mean only "too large to process completely", while the other UI uses it to mean "large enough to partially collapse for viewer convenience".