Fix a race between Harbormaster and reviewers (often bots) to publish drafts for review

Summary:
See PHI309. There is a window of time between when all builds pass and when Harbormaster actually publishes a revision out of draft.

If any other user tries to interact with the revision during that window, they'll pick up the undraft transaction as a side effect. However, they won't have permission to apply it and will be stopped by a validation error.

Instead, only automatically publish a revision if the actor is the revision author or some system/application user (essentially always Harbormaster).

Test Plan:
  - Added a `echo ...; sleep(30);` to `HarbormasterBuildEngine->updateBuildable()` before the `applyTransactions()` at the bottom.
  - Wrote an "Always, run an HTTP request" Herald rule and Harbormaster build plan.
  - Ran daemons with `bin/phd debug task`.
  - Created a new revision with `arc diff`, as user A.
  - Waited for `phd` to enter the race window.
  - In a separate browser, as user B, submitted a comment via `differential.revision.edit`.
    - Before patch: edits during the race window were rejected with a validation error, "you don't have permission to request review".
    - After patch: edits go through cleanly.

Reviewers: amckinley

Reviewed By: amckinley

Differential Revision: https://secure.phabricator.com/D18921

Author: epriestley

src/applications/differential/editor/DifferentialTransactionEditor.php

@@ -1565,17 +1565,36 @@ private function loadUnbroadcastTransactions($object) {
 
 
   protected function didApplyTransactions($object, array $xactions) {
+    // In a moment, we're going to try to publish draft revisions which have
+    // completed all their builds. However, we only want to do that if the
+    // actor is either the revision author or an omnipotent user (generally,
+    // the Harbormaster application).
+
+    // If we let any actor publish the revision as a side effect of other
+    // changes then an unlucky third party who innocently comments on the draft
+    // can end up racing Harbormaster and promoting the revision. At best, this
+    // is confusing. It can also run into validation problems with the "Request
+    // Review" transaction. See PHI309 for some discussion.
+    $author_phid = $object->getAuthorPHID();
+    $viewer = $this->requireActor();
+    $can_undraft =
+      ($this->getActingAsPHID() === $author_phid) ||
+      ($viewer->isOmnipotent());
+
     // If a draft revision has no outstanding builds and we're automatically
     // making drafts public after builds finish, make the revision public.
-    $auto_undraft = !$object->getHoldAsDraft();
+    if ($can_undraft) {
+      $auto_undraft = !$object->getHoldAsDraft();
+    } else {
+      $auto_undraft = false;
+    }
 
     if ($object->isDraft() && $auto_undraft) {
       $active_builds = $this->hasActiveBuilds($object);
       if (!$active_builds) {
         // When Harbormaster moves a revision out of the draft state, we
         // attribute the action to the revision author since this is more
         // natural and more useful.
-        $author_phid = $object->getAuthorPHID();
 
         // Additionally, we change the acting PHID for the transaction set
         // to the author if it isn't already a user so that mail comes from