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.
Details
Details
- Reviewers
joshuaspence - Group Reviewers
Blessed Reviewers - Commits
- rP0ed496de2282: Throw an exception if `local.json` can't be read
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"
}
]
}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]
Diff Detail
Diff Detail
- Repository
- rP Phabricator
- Branch
- err1
- Lint
Lint Passed - Unit
Tests Passed - Build Status
Buildable 17028 Build 22769: Run Core Tests Build 22768: arc lint + arc unit
Event Timeline
Comment Actions
I think we possibly end up with a chicken-and-egg problem here where you can't bin/config to fix things if your local.json is broken, but that's probably fine.
I'm happy to bring a version of this upstream, but it should probably Filesystem::readFile(...), phutil_json_decode(), and distinguish between "can not read" and "read it fine, but your JSON is junk".
Comment Actions
I was thinking about this, but I figured there was probably some good reason for this code using @file_* instead of Filesystem.
Comment Actions
I don't recall a specific reason for continue-on-failure -- it looks like this came from D4294; I think I was just being cautious about things since the path was a bit tentative and there were some potential bootstrapping issues. I think we're on firm ground now and can't come up with any reason not to fail loudly here.
Comment Actions
epriestley@orbital ~/dev/phabricator $ ./bin/worker execute --id 12345678 Usage Exception: No task exists with id "12345678"!
epriestley@orbital ~/dev/phabricator $ chmod -r conf/local/local.json
epriestley@orbital ~/dev/phabricator $ ./bin/worker execute --id 12345678
[2017-05-16 21:57:06] EXCEPTION: (PhutilProxyException) Configuration file "/Users/epriestley/dev/core/lib/phabricator/conf/local/local.json" exists, but could not be read. It may have permissions which are too restrictive. {>} (FilesystemException) Path '/Users/epriestley/dev/core/lib/phabricator/conf/local/local.json' is not readable. at [<phutil>/src/filesystem/Filesystem.php:1124]
arcanist(head=experimental, ref.master=3c4735795a29, ref.experimental=dc65bfbe5434), phabricator(head=master, ref.master=452e6ce51e7e, custom=1), phutil(head=master, ref.master=a900d7b63e95)
#0 <#2> Filesystem::assertReadable(string) called at [<phutil>/src/filesystem/Filesystem.php:39]
#1 <#2> Filesystem::readFile(string) called at [<phabricator>/src/infrastructure/env/PhabricatorConfigLocalSource.php:30]
#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-script.php:9]
#8 require_once(string) called at [<phabricator>/scripts/__init_script__.php:3]
#9 require_once(string) called at [<phabricator>/scripts/setup/manage_worker.php:5]
epriestley@orbital ~/dev/phabricator $ chmod +r conf/local/local.jsonepriestley@orbital ~/dev/phabricator $ echo junk >> conf/local/local.json
epriestley@orbital ~/dev/phabricator $ ./bin/worker execute --id 12345678
[2017-05-16 21:57:31] EXCEPTION: (PhutilProxyException) Configuration file "/Users/epriestley/dev/core/lib/phabricator/conf/local/local.json" 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. {>} (PhutilJSONParserException) Parse error on line 145 at column 1: Expected one of: 'EOF', '}', ',', ']' at [<phutil>/src/parser/PhutilJSONParser.php:32]
arcanist(head=experimental, ref.master=3c4735795a29, ref.experimental=dc65bfbe5434), phabricator(head=master, ref.master=452e6ce51e7e, custom=1), phutil(head=master, ref.master=a900d7b63e95)
#0 <#2> PhutilJSONParser::parse(string) called at [<phutil>/src/utils/utils.php:1154]
#1 <#2> phutil_json_decode(string) called at [<phabricator>/src/infrastructure/env/PhabricatorConfigLocalSource.php:41]
#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-script.php:9]
#8 require_once(string) called at [<phabricator>/scripts/__init_script__.php:3]
#9 require_once(string) called at [<phabricator>/scripts/setup/manage_worker.php:5]Comment Actions
It may have permissions which are too restrictive.
This seems to suggest that the user should make the file world-readable.
Comment Actions
How about this:
Configuration file "%s" exists, but could not be read. It may have permissions which prevent it from being read by the current user.
Although that is sorta saying the same thing twice. So maybe this:
Configuration file "%s" exists, but could not be read. Run the command as a user with read access to this file.
Comment Actions
Although it isn't the case here, my guess is that the most common cause of this error is that the file is owned by epriestley and not readable by www or similar.
If you've done some weird magic to it I'm less concerned about the help text needing to point you in the right direction since you're probably sophisticated enough to identify the remedy ("use sudo" or whatever).
Comment Actions
Run the command as a user with read access to this file.
Yeah, I'm worried this is going to get us some "bug reports" when it shows up in the web UI and users want to know how to run apache as root.
Comment Actions
As in... If users hot this error from non-CLI mode then they probably need to change the file permissions or add their web user to a group. If users hit this from the CLI then they need sudo