v2-hub icon indicating copy to clipboard operation
v2-hub copied to clipboard

Variables in system.yaml handled by .env file are overwritten with value

Open bendeca opened this issue 5 years ago • 2 comments

Setting a variable in the system.yaml config to e.g. '{env:APP_URL}' causes the value to be overwritten with the value of the current .env file. This only happened to me in the system.yaml config and only when editing the license key through cp/licensing.

But looking at the code, I can see that it could happen with any yaml config file.

To Reproduce Steps to reproduce the behavior:

  1. create a .env file in your web root directory with the content TEST=foo
  2. set a param in system.yaml to {env:TEST} e.g. php_max_memory_limit: '{env:TEST}'
  3. Edit the statamic license in the control panel at the url cp/licensing and save !!! this bug does not appear when editing the license key in the config menu > settings > system image
  4. check your system.yaml: It should say:

php_max_memory_limit: foo

Expected behavior variables handled by .env file should never be overwritten via the control panel.

Environment details (please complete the following information):

  • Statamic Version 2.11.17
  • Fresh Install or Upgrade, either
  • OS: macOS 10.15.1
  • Browser: any
  • Web Server: Apache
  • PHP Version: 7.2
  • Addons installed:

FIX The problem lies within statamic/core/Config/Settings.php

The logic is pretty much copied from statamic/core/Http/Controllers/SettingsController.php[58-63] This is also the function that gets called when editing the license key from the settings > system

Fixed content of statamic/core/Config/Settings.php:

<?php

namespace Statamic\Config;

use Statamic\API\File;
use Statamic\API\YAML;

class Settings extends Config
{
    /**
     * Save the config
     *
     * @return void
     */
    public function save()
    {
        foreach ($this->config as $file => $data) {
            // Don't save if it hasn't changed.
            if (array_get($this->original, $file) === $data) {
                continue;
            }

            // Remove environment managed vars from what was submitted, and replace them with their current values.
            // They aren't editable in the CP but will be submitted (possibly incorrectly) anyway.
            $environmentVars = array_keys($this->env()[$file] ?: []);
            $data = array_except($data, $environmentVars);
            $environmentValues = array_only(YAML::parse(File::get(settings_path("$file.yaml"))), $environmentVars);
            $data = array_merge($data, $environmentValues);

            File::put(settings_path("$file.yaml"), YAML::dump($data));
        }
    }
}

bendeca avatar Jan 24 '20 17:01 bendeca

Yes, this is dangerous. I use a .env value for the url of the site in my system.yaml. Then I add my license key through the CP and this get's overwritten with the current local url. If I deploy that the site is broken. I've had that happen multiple times.

studio1902 avatar Apr 22 '20 07:04 studio1902

When I save the cache settings page it overwrites my static_caching_exclude causing big issues for contact form. This is when I use env vars to set the cache settings.

Probably related to the above so thats why I write it here.

stache_always_update: '{env:STACHE_ALWAYS_UPDATE}'
static_caching_enabled: '{env:STATIC_CACHING_ENABLED}'
stache_lock_enabled: true
stache_lock_wait_length: 30
static_caching_lock_hold_length: 0
static_caching_type: '{env:STATIC_CACHING_TYPE}'
static_caching_ignore_query_strings: '{env:STATIC_CACHING_IGNORE_QUERY_STRINGS}'
static_caching_file_path: public/static
static_caching_invalidation: all
static_caching_exclude:
  - /contact-us

philipboomy avatar Jul 02 '20 12:07 philipboomy