2018 Week 1 (Early January)

Updated 2,298 Days AgoPublic

Actions

Summary of changes from January 1, 2018 to January 6, 2018.

• These changes were promoted to stable.

General

Phabricator now prevents "enormous changes" from being pushed to hosted repositories by default. This new protection is similar to the existing protection for "dangerous changes". Both types of protection can be disabled in Manage Repository, or permitted selectively (for example, so that only certain trusted pushers can make these kinds of changes) with Herald rules.

For a long time, Herald has included several commit content fields which can be used to write rules (like [ Diff content added ]) which take action based on the text changes in a commmit: for example, rejecting commits where some added file content matches some pattern.

However, for very large commits, the commit text may not fit in memory. Currently, we can not search for patterns in strings which do not fit in memory. Herald's behavior is to classify these commits as "enormous" and unconditionally fail content rules against them. This is well-defined, but can be surprising (or worse).

Phabricator defines "enormous changes" as commits with too much text to process. The current, specific implementation considers a change "enormous" when the size of the diff is more than 1GB or it takes more than 15 minutes to extract the diff from the underlying VCS. In git, this roughly means that we run a command like this (for other version control systems, we run some equivalent command):

$ git diff <options> -- <commit>^ <commit>

If, for some <commit>, this command takes more than 15 minutes to execute or produces more than 1GB of output, we abort processing of content rules and classify the commit as "enormous" instead.

Herald has had the ability to act on enormous commits with the [ Diff is enormous ] field for a very long time, so it has been possible to write safe, complete rulesets by combining [ Diff is enormous ] rules with content rules. However, this isn't clear or obvious, and it isn't obvious when you've written a rule which can be bypassed by adding 1GB of unrelated text changes to a commit which triggers the rule.

Some of this implementation predates repository hosting and comes from a time when Herald rules were less powerful and commit rules were almost always used in an advisory way. In a modern environment, Herald and Diffusion are both more powerful than they were at this time, and it is less likely that this behavior is harmless. Particularly, the recent [ Commit uses Git LFS ] field is fundamentally a content field and can not detect the use of LFS in enormous commits.

Since these commits are unusual and often either mistakes (accidentally adding an enormous logfile) or special cases (an initial import of an existing project) anyway, we now reject them by default. This makes Herald rules do the obvious thing unless you disable this safeguard.

If you choose to disable this safeguard, the existing [ Diff is enormous ] field can still be used to selectively control the ability to push these changes or write content rules which react to enormous changes in an explicit way.

See T13031 for some more detail, examples, and context.

Security

• The "Enormous Changes" change described above has some security impact. See T13031 for details.
• See T13037 for discussion of a recent security disclosure by Mailgun, a supported mail transmission service.
• "Spectre" and "Meltdown" are in the news, but generally do not impact Phabricator. See T13038 for "clever" jokes about this.

Migrations

• No migrations in this period.

Upgrading / Compatibility

• The "Enormous Changes" change described above changes the default behavior when pushing huge commits to hosted repositories.

Minor

• Fixed a bug where review request mail could be dropped if prototypes are enabled but Harbormaster is not used to trigger builds.
• [] Slightly reduced the level of bleeding/explosions on the Maniphest burnup chart.
• [] Fixed a data frame repackaging issue which could cause pushes to fail intermittently with more recent versions of Mercurial. See T13036 for details.
• [] The drydock.blueprint.edit API method is now a little less unusual.

The [] icon indicates a change backed by support mana.