dovecot: fix quoting for sed in docker-entrypoint

[sandzwerg]

using sed with " instead of ' can fail at all chars that the shell will interpret. This could lead to non starting containers i.e. if the password contained special chars

[Shadowghost]

Depending on what mailcow does this may be overridden when I merge upstream next time. IMHO you should propose those changes on upstream.

[andryyy]

This is supposed to be replaced... :)

[Shadowghost]

I'll wait for upstream but leave this open just in case :)

[andryyy]

This cannot be changed upstream, it will not replace the variable anymore. :)

[sandzwerg]

Then I'd propose to either use a different mechanism to insert these values or add a big disclaimer at the place where these variables are configured

[andryyy]

I think you misunderstand the script.

It does exactly what it should. It needs to replace these values.

[sandzwerg]

Yes, I understand that part. But then it should be made sure that the values that will be used to replace the placeholders don't cause issues. While the generate_config.sh does that, people might choose to create their own passwords. So a comment in docker-compose in the style of:

     # if you generate your own passwords instead of using the generate_config.sh make sure
     # to only use "a-zA-Z0-9" else your container could break
      environment:
        - TZ=${TZ}
...

[andryyy]

The values won't get replaced with '

It is exactly working as intended.

[andryyy]

Besides this change breaking variable substitution and therefore the script, we already mention it in the .env file: https://github.com/mailcow/mailcow-dockerized/blob/master/generate_config.sh#L132

[sandzwerg]

I'm not sure we're taling about the same stuff. The docker-entrypoint.sh for dovecot includes some sed scripts that replace placeholders like __DBPASS__ with the values from the enviroment variable ${DBPASS} see https://github.com/mailcow/mailcow-dockerized/blob/master/data/Dockerfiles/dovecot/docker-entrypoint.sh#L270 ff This will break if one uses single backticks instead of double quotes as you correctly pointed out (because the variable won't get evaluated). But this will break if the ${DBPASS} (or similar) variables contain characters that get evaluated by the shell, which could happen if people chose to generate a password on their own.

I just saw that the script already includes a line regarding the password https://github.com/mailcow/mailcow-dockerized/blob/master/generate_config.sh#L132 but at least to me it wasn't clear that it would break if someone chooses a "better" password i.e. one which might include special chars. Maybe the wording here could be a bit stronger? I agree however that this PR should be closed.