Page MenuHomePhabricator
Files F13232119

D20159.diff
No OneTemporary
Actions

D20159.diff
View Options

diff --git a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
--- a/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
+++ b/src/applications/config/check/PhabricatorExtraConfigSetupCheck.php
@@ -15,6 +15,9 @@
$defined_keys = PhabricatorApplicationConfigOptions::loadAllOptions();
+ $stack = PhabricatorEnv::getConfigSourceStack();
+ $stack = $stack->getStack();
+
foreach ($all_keys as $key) {
if (isset($defined_keys[$key])) {
continue;
@@ -48,9 +51,6 @@
->setName($name)
->setSummary($summary);
- $stack = PhabricatorEnv::getConfigSourceStack();
- $stack = $stack->getStack();
-
$found = array();
$found_local = false;
$found_database = false;
@@ -85,6 +85,101 @@
}
}
+ $options = PhabricatorApplicationConfigOptions::loadAllOptions();
+ foreach ($defined_keys as $key => $value) {
+ $option = idx($options, $key);
+ if (!$option) {
+ continue;
+ }
+
+ if (!$option->getLocked()) {
+ continue;
+ }
+
+ $found_database = false;
+ foreach ($stack as $source_key => $source) {
+ $value = $source->getKeys(array($key));
+ if ($value) {
+ if ($source instanceof PhabricatorConfigDatabaseSource) {
+ $found_database = true;
+ break;
+ }
+ }
+ }
+
+ if (!$found_database) {
+ continue;
+ }
+
+ // NOTE: These are values which we don't let you edit directly, but edit
+ // via other UI workflows. For now, don't raise this warning about them.
+ // In the future, before we stop reading database configuration for
+ // locked values, we either need to add a flag which lets these values
+ // continue reading from the database or move them to some other storage
+ // mechanism.
+ $soft_locks = array(
+ 'phabricator.uninstalled-applications',
+ 'phabricator.application-settings',
+ 'config.ignore-issues',
+ );
+ $soft_locks = array_fuse($soft_locks);
+ if (isset($soft_locks[$key])) {
+ continue;
+ }
+
+ $doc_name = 'Configuration Guide: Locked and Hidden Configuration';
+ $doc_href = PhabricatorEnv::getDoclink($doc_name);
+
+ $set_command = phutil_tag(
+ 'tt',
+ array(),
+ csprintf(
+ 'bin/config set %R <value>',
+ $key));
+
+ $summary = pht(
+ 'Configuration value "%s" is locked, but has a value in the database.',
+ $key);
+ $message = pht(
+ 'The configuration value "%s" is locked (so it can not be edited '.
+ 'from the web UI), but has a database value. Usually, this means '.
+ 'that it was previously not locked, you set it using the web UI, '.
+ 'and it later became locked.'.
+ "\n\n".
+ 'You should copy this configuration value in a local configuration '.
+ 'source (usually by using %s) and then remove it from the database '.
+ 'with the command below.'.
+ "\n\n".
+ 'For more information on locked and hidden configuration, including '.
+ 'details about this setup issue, see %s.'.
+ "\n\n".
+ 'This database value is currently respected, but a future version '.
+ 'of Phabricator will stop respecting database values for locked '.
+ 'configuration options.',
+ $key,
+ $set_command,
+ phutil_tag(
+ 'a',
+ array(
+ 'href' => $doc_href,
+ 'target' => '_blank',
+ ),
+ $doc_name));
+ $command = csprintf(
+ 'phabricator/ $ ./bin/config delete --database %R',
+ $key);
+
+ $this->newIssue('config.locked.'.$key)
+ ->setShortName(pht('Deprecated Config Source'))
+ ->setName(
+ pht(
+ 'Locked Configuration Option "%s" Has Database Value',
+ $key))
+ ->setSummary($summary)
+ ->setMessage($message)
+ ->addCommand($command)
+ ->addPhabricatorConfig($key);
+ }
if (PhabricatorEnv::getEnvConfig('feed.http-hooks')) {
$this->newIssue('config.deprecated.feed.http-hooks')
diff --git a/src/applications/config/option/PhabricatorPHDConfigOptions.php b/src/applications/config/option/PhabricatorPHDConfigOptions.php
--- a/src/applications/config/option/PhabricatorPHDConfigOptions.php
+++ b/src/applications/config/option/PhabricatorPHDConfigOptions.php
@@ -41,7 +41,12 @@
"If you are running a cluster, this limit applies separately ".
"to each instance of `phd`. For example, if this limit is set ".
"to `4` and you have three hosts running daemons, the effective ".
- "global limit will be 12.")),
+ "global limit will be 12.".
+ "\n\n".
+ "After changing this value, you must restart the daemons. Most ".
+ "configuration changes are picked up by the daemons ".
+ "automatically, but pool sizes can not be changed without a ".
+ "restart.")),
$this->newOption('phd.verbose', 'bool', false)
->setLocked(true)
->setBoolOptions(
diff --git a/src/docs/user/configuration/configuration_locked.diviner b/src/docs/user/configuration/configuration_locked.diviner
--- a/src/docs/user/configuration/configuration_locked.diviner
+++ b/src/docs/user/configuration/configuration_locked.diviner
@@ -111,6 +111,55 @@
```
+Locked Configuration With Database Values
+=========================================
+
+You may receive a setup issue warning you that a locked configuration key has a
+value set in the database. Most commonly, this is because:
+
+ - In some earlier version of Phabricator, this configuration was not locked.
+ - In the past, you or some other administrator used the web UI to set a
+ value. This value was written to the database.
+ - In a later version of the software, the value became locked.
+
+When Phabricator was originally released, locked configuration did not yet
+exist. Locked configuration was introduced later, and then configuration options
+were gradually locked for a long time after that.
+
+In some cases the meaning of a value changed and it became possible to use it
+to break an install or the configuration became a security risk. In other
+cases, we identified an existing security risk or arrived at some other reason
+to lock the value.
+
+Locking values was more common in the past, and it is now relatively rare for
+an unlocked value to become locked: when new values are introduced, they are
+generally locked or hidden appropriately. In most cases, this setup issue only
+affects installs that have used Phabricator for a long time.
+
+At time of writing (February 2019), Phabricator currently respects these old
+database values. However, some future version of Phabricator will refuse to
+read locked configuration from the database, because this improves security if
+an attacker manages to find a way to bypass restrictions on editing locked
+configuration from the web UI.
+
+To clear this setup warning and avoid surprise behavioral changes in the future,
+you should move these configuration values from the database to a local config
+file. Usually, you'll do this by first copying the value from the database:
+
+```
+phabricator/ $ ./bin/config set <key> <value>
+```
+
+...and then removing the database value:
+
+```
+phabricator/ $ ./bin/config delete --database <key>
+```
+
+See @{Configuration User Guide: Advanced Configuration} for some more detailed
+discussion of different configuration sources.
+
+
Next Steps
==========

File Metadata

Mime Type
text/plain
Expires
May 22 2024, 1:02 AM (4 w, 5 d ago)
Storage Engine
blob
Storage Format
Encrypted (AES-256-CBC)
Storage Handle
6299889
Default Alt Text
D20159.diff (7 KB)

Event Timeline

Phabricator · Built by Phacility