Move open_basedir and safe_mode checks into new setup

Summary: Newer and shinier!

Test Plan: Intentionally misconfigured myself into all three setup failures (safe_mode, open_basedir/fatal, open_basedir/nonfatal).

Reviewers: chad, btrahan

Reviewed By: btrahan

CC: aran

Maniphest Tasks: T2228

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

src/__phutil_library_map__.php

@@ -1221,6 +1221,7 @@ phutil_register_library_map(array(
     'PhabricatorSetupCheckInvalidConfig' => 'applications/config/check/PhabricatorSetupCheckInvalidConfig.php',
     'PhabricatorSetupCheckMail' => 'applications/config/check/PhabricatorSetupCheckMail.php',
     'PhabricatorSetupCheckMySQL' => 'applications/config/check/PhabricatorSetupCheckMySQL.php',
+    'PhabricatorSetupCheckPHPConfig' => 'applications/config/check/PhabricatorSetupCheckPHPConfig.php',
     'PhabricatorSetupCheckPath' => 'applications/config/check/PhabricatorSetupCheckPath.php',
     'PhabricatorSetupCheckStorage' => 'applications/config/check/PhabricatorSetupCheckStorage.php',
     'PhabricatorSetupCheckTimezone' => 'applications/config/check/PhabricatorSetupCheckTimezone.php',
@@ -2586,6 +2587,7 @@ phutil_register_library_map(array(
     'PhabricatorSetupCheckInvalidConfig' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckMail' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckMySQL' => 'PhabricatorSetupCheck',
+    'PhabricatorSetupCheckPHPConfig' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckPath' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckStorage' => 'PhabricatorSetupCheck',
     'PhabricatorSetupCheckTimezone' => 'PhabricatorSetupCheck',

src/applications/config/check/PhabricatorSetupCheckPHPConfig.php

@@ -0,0 +1,113 @@
+<?php
+
+final class PhabricatorSetupCheckPHPConfig extends PhabricatorSetupCheck {
+
+  public function getExecutionOrder() {
+    return 0;
+  }
+
+  protected function executeChecks() {
+    $safe_mode = ini_get('safe_mode');
+    if ($safe_mode) {
+      $message = pht(
+        "You have 'safe_mode' enabled in your PHP configuration, but ".
+        "Phabricator will not run in safe mode. Safe mode has been deprecated ".
+        "in PHP 5.3 and removed in PHP 5.4.".
+        "\n\n".
+        "Disable safe mode to continue.");
+
+      $this->newIssue('php.safe_mode')
+        ->setIsFatal(true)
+        ->setName(pht('Disable PHP safe_mode'))
+        ->setMessage($message)
+        ->addPHPConfig('safe_mode');
+      return;
+    }
+
+    $open_basedir = ini_get('open_basedir');
+    if ($open_basedir) {
+
+      // 'open_basedir' restricts which files we're allowed to access with
+      // file operations. This might be okay -- we don't need to write to
+      // arbitrary places in the filesystem -- but we need to access certain
+      // resources. This setting is unlikely to be providing any real measure
+      // of security so warn even if things look OK.
+
+      $failures = array();
+
+      try {
+        $open_libphutil = class_exists('Future');
+      } catch (Exception $ex) {
+        $failures[] = $ex->getMessage();
+      }
+
+      try {
+        $open_arcanist = class_exists('ArcanistDiffParser');
+      } catch (Exception $ex) {
+        $failures[] = $ex->getMessage();
+      }
+
+      $open_urandom = false;
+      try {
+        Filesystem::readRandomBytes(1);
+        $open_urandom = true;
+      } catch (FilesystemException $ex) {
+        $failures[] = $ex->getMessage();
+      }
+
+      try {
+        $tmp = new TempFile();
+        file_put_contents($tmp, '.');
+        $open_tmp = @fopen((string)$tmp, 'r');
+        if (!$open_tmp) {
+          $failures[] = pht(
+            "Unable to read temporary file '%s'.",
+            (string)$tmp);
+        }
+      } catch (Exception $ex) {
+        $message = $ex->getMessage();
+        $dir = sys_get_temp_dir();
+        $failures[] = pht(
+          "Unable to open temp files from '%s': %s",
+          $dir,
+          $message);
+      }
+
+      $issue = $this->newIssue('php.open_basedir')
+        ->setName(pht('Disable PHP open_basedir'))
+        ->addPHPConfig('open_basedir');
+
+      if ($failures) {
+        $message = pht(
+          "Your server is configured with 'open_basedir', which prevents ".
+          "Phabricator from opening files it requires access to.".
+          "\n\n".
+          "Disable this setting to continue.".
+          "\n\n".
+          "Failures:\n\n%s",
+          implode("\n\n", $failures));
+
+        $issue
+          ->setIsFatal(true)
+          ->setMessage($message);
+
+        return;
+      } else {
+        $summary = pht(
+          "You have 'open_basedir' configured in your PHP settings, which ".
+          "may cause some features to fail.");
+
+        $message = pht(
+          "You have 'open_basedir' configured in your PHP settings. Although ".
+          "this setting appears permissive enough that Phabricator will ".
+          "work properly, you may still run into problems because of it.".
+          "\n\n".
+          "Consider disabling 'open_basedir'.");
+
+        $issue
+          ->setSummary($summary)
+          ->setMessage($message);
+      }
+    }
+  }
+}

src/infrastructure/PhabricatorSetup.php

@@ -15,90 +15,6 @@ final class PhabricatorSetup {
 
     self::writeHeader("CORE CONFIGURATION");
 
-    // NOTE: Test this first since other tests depend on the ability to
-    // execute system commands and will fail if safe_mode is enabled.
-    $safe_mode = ini_get('safe_mode');
-    if ($safe_mode) {
-      self::writeFailure();
-      self::write(
-        "Setup failure! You have 'safe_mode' enabled. Phabricator will not ".
-        "run in safe mode, and it has been deprecated in PHP 5.3 and removed ".
-        "in PHP 5.4.\n");
-      return;
-    } else {
-      self::write(" okay  PHP's deprecated 'safe_mode' is disabled.\n");
-    }
-
-    // NOTE: Also test this early since we can't include files from other
-    // libraries if this is set strictly.
-
-    $open_basedir = ini_get('open_basedir');
-    if ($open_basedir) {
-
-      // 'open_basedir' restricts which files we're allowed to access with
-      // file operations. This might be okay -- we don't need to write to
-      // arbitrary places in the filesystem -- but we need to access certain
-      // resources. This setting is unlikely to be providing any real measure
-      // of security so warn even if things look OK.
-
-      try {
-        $open_libphutil = class_exists('Future');
-      } catch (Exception $ex) {
-        $message = $ex->getMessage();
-        self::write("Unable to load modules from libphutil: {$message}\n");
-        $open_libphutil = false;
-      }
-
-      try {
-        $open_arcanist = class_exists('ArcanistDiffParser');
-      } catch (Exception $ex) {
-        $message = $ex->getMessage();
-        self::write("Unable to load modules from Arcanist: {$message}\n");
-        $open_arcanist = false;
-      }
-
-      $open_urandom = false;
-      try {
-        Filesystem::readRandomBytes(1);
-        $open_urandom = true;
-      } catch (FilesystemException $ex) {
-        self::write($ex->getMessage()."\n");
-      }
-
-      try {
-        $tmp = new TempFile();
-        file_put_contents($tmp, '.');
-        $open_tmp = @fopen((string)$tmp, 'r');
-      } catch (Exception $ex) {
-        $message = $ex->getMessage();
-        $dir = sys_get_temp_dir();
-        self::write("Unable to open temp files from '{$dir}': {$message}\n");
-        $open_tmp = false;
-      }
-
-      if (!$open_urandom || !$open_tmp || !$open_libphutil || !$open_arcanist) {
-        self::writeFailure();
-        self::write(
-          "Setup failure! Your server is configured with 'open_basedir' in ".
-          "php.ini which prevents Phabricator from opening files it needs to ".
-          "access. Either make the setting more permissive or remove it. It ".
-          "is unlikely you derive significant security benefits from having ".
-          "this configured; files outside this directory can still be ".
-          "accessed through system command execution.");
-        return;
-      } else {
-        self::write(
-          "[WARN] You have an 'open_basedir' configured in your php.ini. ".
-          "Although the setting seems permissive enough that Phabricator ".
-          "will run properly, you may run into problems because of it. It is ".
-          "unlikely you gain much real security benefit from having it ".
-          "configured, because the application can still access files outside ".
-          "the 'open_basedir' by running system commands.\n");
-      }
-    } else {
-      self::write(" okay  'open_basedir' is not set.\n");
-    }
-
     if (!PhabricatorEnv::getEnvConfig('security.alternate-file-domain')) {
       self::write(
         "[WARN] You have not configured 'security.alternate-file-domain'. ".