Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dealing with aliases and environment variables containing special characters ($, @) #3769

Closed
Mosnar opened this issue Feb 1, 2019 · 3 comments

Comments

@Mosnar
Copy link
Contributor

Mosnar commented Feb 1, 2019

Description

We store our database credentials, among other things, in the environment and use auto-generated passwords. This has caused an issue with the move to autosuggest fields and the use of environment variables/aliases in the CP since frequently, our passwords contain or even start with "@" or "$". This causes a pretty nasty error in the CP displaying the password in question:

image

I'm not sure what the best way around this is, but disallowing these characters in passwords/forcing people to change passwords to less secure ones doesn't feel like the right approach.

Steps to reproduce

  1. Use a password in env with an "@" or "$"
  2. Try to use an autosuggest field in the CP

Additional info

  • Craft version: 3.1.7
  • PHP version: 7.2.13
  • Database driver & version: N/A
  • Plugins & versions: N/A
@brandonkelly
Copy link
Member

This seems like a duplicate of #3700, which we fixed in 3.1.4. That $throwException variable in the condition shown in your screenshot is set to false now (b40d9c7).

You sure you’re actually running 3.1.7? Check line 85 of vendor/craftcms/cms/src/Craft.php and confirm it looks like this:

return static::getAlias($str, false) ?: $str;

If not, there may have been an issue when updating previously. Try nuking your vendor/ folder and run composer update.

If it does, then maybe it’s a opcache issue?

@Mosnar
Copy link
Contributor Author

Mosnar commented Feb 1, 2019

You're right! After further inspection, it looks like this is specifically affecting my S3 Volumes rather than all of them as I previously thought. It looks like S3's Volume.php is still using parseEnv without silencing exceptions:

Craft::parseEnv($this->cfDistributionId),

I'm guessing this was just introduced as a bug with the release of 1.1.1

Edit
On second thought, that doesn't fully explain things, since the silencing happens within parseEnv(), so it should be silenced anyway.

Edit 2
It looks like the issue is from within the Cp Twig method: getEnvSuggestions() where it's calling getAlias directly without silencing exceptions:

craftcms/cms/src/web/twig/variables/Cp.php:333

foreach (array_keys($_ENV) as $var) {
            $envSuggestions[] = [
                'name' => '$' . $var,
                'hint' => $security->redactIfSensitive($var, Craft::getAlias(getenv($var)))
            ];
        }

@brandonkelly
Copy link
Member

Ahhh, thanks for looking into it! Just fixed that for the next release.

You can patch your install to get the fix now by changing your craftcms/cms requirement in composer.json to:

"require": {
    "craftcms/cms": "dev-develop#0bf918445ea2db832bdbd843705dfc08d05a678d as 3.1.7",
    "...": "..."
}

Then run composer update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants