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

Add config files to protected_web_paths in pantheon.yml #40

Open
jenlampton opened this issue Jul 21, 2020 · 2 comments · May be fixed by #57
Open

Add config files to protected_web_paths in pantheon.yml #40

jenlampton opened this issue Jul 21, 2020 · 2 comments · May be fixed by #57

Comments

@jenlampton
Copy link
Member

We're still using core's default behavior of naming the config directory by using an md5 hash of the database connection information, which is security-by-obscurity:

$config_directories['active'] = 'files/config_' . md5($database) . '/active';
$config_directories['staging'] = 'files/config_' . md5($database) . '/staging';

Pantheon now provides a protected_web_paths option in the pantheon.yml that will more securely prevent anyone from being able to access these files.

For new installs, I'd like to recommend that we include files/config in the list of protected paths, and replace the config locations as follows:

$config_directories['active'] = 'files/config/active';
$config_directories['staging'] = 'files/config/staging';

Would it be possible to make such a change without affecting current sites?

@herbdool
Copy link
Contributor

I agree it would be good to switch to using protected paths. Though we'll need a strategy here because I think it'll change it for existing sites too. Not sure of best approach. Maybe an update hook to move the config, if possible. Or have a conditional to check for config in the old location if the new one fails.

@herbdool
Copy link
Contributor

I've added a PR. I think it'll do the trick and still be backwards-compatible. I haven't tested so I'm not sure if the random directory will be created in a regular install or if it'll skip that. So needs testing before we can merge.

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

Successfully merging a pull request may close this issue.

2 participants