do not render env variables in configs

[pjfanning]

see lightbend/config#798

The Akka team issued CVE-2023-45865 for the equivalent of our change but the general consensus on Pekko side is that printing the configs to the logs is not the default behaviour. It is useful to make a change but we don't think this warrants a CVE.

We strongly recommend that log-config-on-start is not enabled in production environments.

[He-Pin]

lgtm, do we need to change the code in spec too?

[mdedetrich]

Can we add a simple unit test for this?

[pjfanning]

I can modify the code to make it easier to test.

[pjfanning]

@mdedetrich it looks like the only real way to test this is to set an environment variable. I don't think I can set this in a test itself. I think I would need to set the env variable before calling sbt. I could change one or more of our CI jobs to set a test env var and use it in the test. I can get the test to pass if someone hasn't set this env variable so that we don't make it more complicated for people to run the tests manually.

[mdedetrich]

If it requires too much work we can skip it I guess, its just usually better to have a test for these kinds of things. Isn't sys.env mutable which might make things easier?

[pjfanning]

If it requires too much work we can skip it I guess, its just usually better to have a test for these kinds of things. Isn't sys.env mutable which might make things easier?

I can play around with sys.env and see if that work. I also contacted you in Slack about another related issue that I don't want to make public yet.

[pjfanning]

@mdedetrich It looks like sys env is read only - likewise with System.getenv(). We might need to rely on whether config jar is well tested. I have also added my own code to redact username.

[mdedetrich]

@pjfanning Okay, just ping me when the PR is ready. If its not possible to test then I would just go with the initial implementation (iirc it was simpler)

[pjfanning]

@mdedetrich I've kept the centralised method for rendering with redactions. My code does an additional redaction of the username and may in future do more. The toString of the config is very large and I wouldn't be surprised if we need to do more changes in future.

[mdedetrich]

Thanks ill check it out tomorrow

[pjfanning]

@mdedetrich would you have time to look at this again? I'm hoping to do a v1.0.2 release within the next few weeks.

[mdedetrich]

Minor nit but otherwise lgtm