From 6857ffb6f59187b65709efe7d9e91b5c53658de2 Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Fri, 28 Jun 2013 17:23:36 -0700
Subject: [PATCH] Validate all components of $PATH configuration

Summary: Fixes T3400. Users are crafty. Attempt to outwit them.

Test Plan: Added all kinds of nonsense to my PATH to hit all the errors. Verified sensible-looking error messages which I couldn't figure out any way to misread or outwit.

Reviewers: chad, btrahan

Reviewed By: chad

CC: aran

Maniphest Tasks: T3400

Differential Revision: https://secure.phabricator.com/D6318
---
 .../check/PhabricatorSetupCheckPath.php       | 95 +++++++++++++++++++
 1 file changed, 95 insertions(+)

diff --git a/src/applications/config/check/PhabricatorSetupCheckPath.php b/src/applications/config/check/PhabricatorSetupCheckPath.php
index 253872ae73..df72d52f20 100644
--- a/src/applications/config/check/PhabricatorSetupCheckPath.php
+++ b/src/applications/config/check/PhabricatorSetupCheckPath.php
@@ -25,6 +25,101 @@ final class PhabricatorSetupCheckPath extends PhabricatorSetupCheck {
         ->setSummary($summary)
         ->setMessage($message)
         ->addPhabricatorConfig('environment.append-paths');
+
+      // Bail on checks below.
+      return;
     }
+
+    // Users are remarkably industrious at misconfiguring software. Try to
+    // catch mistaken configuration of PATH.
+
+    $path_parts = explode(PATH_SEPARATOR, $path);
+    $bad_paths = array();
+    foreach ($path_parts as $path_part) {
+      if (!strlen($path_part)) {
+        continue;
+      }
+
+      $message = null;
+      $not_exists = false;
+      foreach (Filesystem::walkToRoot($path_part) as $part) {
+        if (!Filesystem::pathExists($part)) {
+          $not_exists = $part;
+          // Walk up so we can tell if this is a readability issue or not.
+          continue;
+        } else if (!is_dir(Filesystem::resolvePath($part))) {
+          $message = pht(
+            "The PATH component '%s' (which resolves as the absolute path ".
+            "'%s') is not usable because '%s' is not a directory.",
+            $path_part,
+            Filesystem::resolvePath($path_part),
+            $part);
+        } else if (!is_readable($part)) {
+          $message = pht(
+            "The PATH component '%s' (which resolves as the absolute path ".
+            "'%s') is not usable because '%s' is not readable.",
+            $path_part,
+            Filesystem::resolvePath($path_part),
+            $part);
+        } else if ($not_exists) {
+          $message = pht(
+            "The PATH component '%s' (which resolves as the absolute path ".
+            "'%s') is not usable because '%s' does not exist.",
+            $path_part,
+            Filesystem::resolvePath($path_part),
+            $not_exists);
+        } else {
+          // Everything seems good.
+          break;
+        }
+
+        if ($message !== null) {
+          break;
+        }
+      }
+
+      if ($message === null) {
+        if (!phutil_is_windows() && !@file_exists($path_part.'/.')) {
+          $message = pht(
+            "The PATH component '%s' (which resolves as the absolute path ".
+            "'%s') is not usable because it is not traversable (its '+x' ".
+            "permission bit is not set).",
+            $path_part,
+            Filesystem::resolvePath($path_part));
+        }
+      }
+
+      if ($message !== null) {
+        $bad_paths[$path_part] = $message;
+      }
+    }
+
+    if ($bad_paths) {
+      foreach ($bad_paths as $path_part => $message) {
+        $digest = substr(PhabricatorHash::digest($path_part), 0, 8);
+
+        $this
+          ->newIssue('config.PATH.'.$digest)
+          ->setName(pht('$PATH Compontent Unusable'))
+          ->setSummary(
+            pht(
+              "A component of the configured PATH can not be used by ".
+              "the webserver: %s",
+              $path_part))
+          ->setMessage(
+            pht(
+              "The configured PATH includes a component which is not usable. ".
+              "Phabricator will be unable to find or execute binaries located ".
+              "here:".
+              "\n\n".
+              "%s".
+              "\n\n".
+              "The user that the webserver runs as must be able to read all ".
+              "the directories in PATH in order to make use of them.",
+              $message))
+          ->addPhabricatorConfig('environment.append-paths');
+      }
+    }
+
   }
 }