- Issue created by @o'briat
- 🇬🇧United Kingdom adamps
$settings['mailer_sendmail_commands']
is a security mechanism. It is an array of allowed commands that are then available in the UI to choose from. This avoids the situation where the UI admin can run any arbitrary shell command. So you can put both of your possible commands into the array insettings.local.php
, and then configure a different one in prod/dev. This is already documented at https://www.drupal.org/docs/contributed-modules/drupal-symfony-mailer/ge... → .Override of config in
settings.local.php
is a completely separate thing. It's standard (not specific to this module), and well known, so I feel it doesn't need to be documented specifically to this module.So I'm not clear any doc change is needed. Does that make sense?
- 🇫🇷France o'briat Nantes
OK, thanks for the details.
But there are several reasons for not committing system commands in the config files : these data could be sensitives, only known to sysadmin, changed frequently or dynamically generated on deployment.
Why not using a keyed array?
Ex:$settings['mailer_sendmail_commands'] = [ 'default' => '/usr/local/bin/sendmail', 'Mail pit' => '/usr/local/bin/mailpit sendmail -t --smtp-addr 127.0.0.1:1025 -t', 'sendmail (internal)' => '/usr/local/bin/intermail -c config1.conf', 'sendmail (external)' => '/usr/local/bin/extenmail -c config2.conf', ];
That way you can safely commit the key(s) in the config (and even avoid using config split).
If you're agree you could put back this issue status to "Feature request", otherwise feel free to close it.
- 🇬🇧United Kingdom adamps
OK it's an interesting idea. Please can you update the IS and title based on #3?
We would need to be back-compatible and also accept the original way. The "default" one shouldn't be in the list as it will be hard-coded.
The identical mechanism has now been added into Core, and really we ought to keep them the same.
- 🇫🇷France o'briat Nantes
I changed title and description, feel free to edit it.
I'll have a look at the "Experimental Symfony Mailer Module" initiative, it looks like they'll cross the same issue:
$config['system.mail']['mailer_dsn'] = [ 'scheme' => 'sendmail', 'host' => 'default', ]; $config['system.mail']['mailer_dsn'] = [ 'scheme' => 'smtp', 'host' => 'localhost', 'port' => 1025, ];
- 🇬🇧United Kingdom adamps
As I already said in #2,
mailer_sendmail_commands
is a security mechanism to avoid the situation where the UI admin can run any arbitrary shell command and the same mechanism exists in Drupal Core. You have observed that it could be re-used to handle sensitive data if changed in a particular way - and I agree. However it would only work for sendmail transports, and the information is still in plain text (just in a different place), so it's a limited solution.There is already an issue for encrypting sensitive data, #3309471: Encrypt stored remote Transport credentials → which proposes using standard mechanisms and contrib modules designed for this purpose.
When I said "OK it's an interesting idea" - I meant it was interesting to avoid duplicating the very same command in the yaml file and also settings.php. However on the plus side the duplicating provides some extra security ensuring that the command matches. In the end the situation is that the decision has been made - the code is in contrib and core, it's documented in various places and likely 100s of sites are using it. It would be a lot of work to change it, and the benefit seems small. I propose that we postpone this for now and we can re-open if there is a lot of interest, or if Core decides to change.
- 🇫🇷France o'briat Nantes
OK.
I still think that this structure will add a lot of overheads on projects with several environments.