From 0ed496de228271add3a63a7f08f3228d21427ec8 Mon Sep 17 00:00:00 2001
From: Joshua Spence <josh@freelancer.com>
Date: Tue, 16 May 2017 14:53:26 -0700
Subject: [PATCH] Throw an exception if `local.json` can't be read

Summary: Our `local.json` configuration file contains various secrets, including database usernames and passwords. As such, we recently changed the permissions on this file from `0644` to `0640`. After doing so, however, I constantly forget to run commands with `sudo`. This is made worse by the fact that `PhabricatorConfigLocalSource` seems to simply ignore `local.json` is it isn't readable, whereas throwing an `Exception` would have saved me a lot of debugging.

Test Plan:
```name=Before
> /usr/local/src/phabricator/bin/config get mysql.pass
{
  "config": [
    {
      "key": "mysql.pass",
      "source": "local",
      "value": null,
      "status": "unset",
      "errorInfo": null
    },
    {
      "key": "mysql.pass",
      "source": "database",
      "value": null,
      "status": "error",
      "errorInfo": "Database source is not configured properly"
    }
  ]
}
```

```name=After
> /usr/local/src/phabricator/bin/config get mysql.pass
[2017-05-16 21:49:26] EXCEPTION: (FilesystemException) Path '/usr/local/src/phabricator/conf/local/local.json' is not readable. at [<phutil>/src/filesystem/Filesystem.php:1124]
arcanist(head=stable, ref.master=3c4735795a29, ref.stable=20ad47f27331), phabricator(head=stable, ref.master=3dae9701298f, ref.stable=fcebaa5097f3), phutil(head=stable, ref.master=a900d7b63e95, ref.stable=d02cc05931b0)
  #0 Filesystem::assertReadable(string) called at [<phutil>/src/filesystem/Filesystem.php:39]
  #1 Filesystem::readFile(string) called at [<phabricator>/src/infrastructure/env/PhabricatorConfigLocalSource.php:25]
  #2 PhabricatorConfigLocalSource::loadConfig() called at [<phabricator>/src/infrastructure/env/PhabricatorConfigLocalSource.php:6]
  #3 PhabricatorConfigLocalSource::__construct() called at [<phabricator>/src/infrastructure/env/PhabricatorEnv.php:195]
  #4 PhabricatorEnv::buildConfigurationSourceStack(boolean) called at [<phabricator>/src/infrastructure/env/PhabricatorEnv.php:95]
  #5 PhabricatorEnv::initializeCommonEnvironment(boolean) called at [<phabricator>/src/infrastructure/env/PhabricatorEnv.php:75]
  #6 PhabricatorEnv::initializeScriptEnvironment(boolean) called at [<phabricator>/scripts/init/lib.php:22]
  #7 init_phabricator_script(array) called at [<phabricator>/scripts/init/init-setup.php:11]
  #8 require_once(string) called at [<phabricator>/scripts/setup/manage_config.php:5]
```

Reviewers: #blessed_reviewers, joshuaspence

Reviewed By: joshuaspence

Subscribers: Korvin

Differential Revision: https://secure.phabricator.com/D17917
---
 .../env/PhabricatorConfigLocalSource.php      | 36 ++++++++++++++-----
 1 file changed, 27 insertions(+), 9 deletions(-)

diff --git a/src/infrastructure/env/PhabricatorConfigLocalSource.php b/src/infrastructure/env/PhabricatorConfigLocalSource.php
index 3a2295eb5e..16dc43a9bc 100644
--- a/src/infrastructure/env/PhabricatorConfigLocalSource.php
+++ b/src/infrastructure/env/PhabricatorConfigLocalSource.php
@@ -21,17 +21,35 @@ final class PhabricatorConfigLocalSource extends PhabricatorConfigProxySource {
 
   private function loadConfig() {
     $path = $this->getConfigPath();
-    if (@file_exists($path)) {
-      $data = @file_get_contents($path);
-      if ($data) {
-        $data = json_decode($data, true);
-        if (is_array($data)) {
-          return $data;
-        }
-      }
+
+    if (!Filesystem::pathExists($path)) {
+      return array();
     }
 
-    return array();
+    try {
+      $data = Filesystem::readFile($path);
+    } catch (FilesystemException $ex) {
+      throw new PhutilProxyException(
+        pht(
+          'Configuration file "%s" exists, but could not be read.',
+          $path),
+        $ex);
+    }
+
+    try {
+      $result = phutil_json_decode($data);
+    } catch (PhutilJSONParserException $ex) {
+      throw new PhutilProxyException(
+        pht(
+          'Configuration file "%s" exists and is readable, but the content '.
+          'is not valid JSON. You may have edited this file manually and '.
+          'introduced a syntax error by mistake. Correct the file syntax '.
+          'to continue.',
+          $path),
+        $ex);
+    }
+
+    return $result;
   }
 
   private function saveConfig() {