From eb0f2a3613e5c07bcfee6a14f7767afe8a58117d Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 1 Nov 2023 23:12:17 +0100 Subject: [PATCH 1/5] do not render env variables in configs --- .../scala/org/apache/pekko/actor/typed/ActorSystem.scala | 7 ++++--- .../main/scala/org/apache/pekko/actor/ActorSystem.scala | 5 +++-- .../scala/org/apache/pekko/dispatch/Dispatchers.scala | 8 +++++--- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/actor-typed/src/main/scala/org/apache/pekko/actor/typed/ActorSystem.scala b/actor-typed/src/main/scala/org/apache/pekko/actor/typed/ActorSystem.scala index 4cac4e496e8..7f423668d4d 100644 --- a/actor-typed/src/main/scala/org/apache/pekko/actor/typed/ActorSystem.scala +++ b/actor-typed/src/main/scala/org/apache/pekko/actor/typed/ActorSystem.scala @@ -17,9 +17,9 @@ import java.util.concurrent.{ CompletionStage, ThreadFactory } import scala.concurrent.{ ExecutionContextExecutor, Future } -import com.typesafe.config.{ Config, ConfigFactory } -import org.slf4j.Logger +import com.typesafe.config.{ Config, ConfigFactory, ConfigRenderOptions } +import org.slf4j.Logger import org.apache.pekko import pekko.{ actor => classic, Done } import pekko.actor.{ Address, BootstrapSetup, ClassicActorSystemProvider } @@ -324,7 +324,8 @@ final class Settings(val config: Config, val classicSettings: classic.ActorSyste /** * Returns the String representation of the Config that this Settings is backed by */ - override def toString: String = config.root.render + override def toString: String = + config.root.render(ConfigRenderOptions.defaults().setShowEnvVariableValues(false)) private val typedConfig = config.getConfig("pekko.actor.typed") diff --git a/actor/src/main/scala/org/apache/pekko/actor/ActorSystem.scala b/actor/src/main/scala/org/apache/pekko/actor/ActorSystem.scala index f5a113e0780..df1ad2efc6a 100644 --- a/actor/src/main/scala/org/apache/pekko/actor/ActorSystem.scala +++ b/actor/src/main/scala/org/apache/pekko/actor/ActorSystem.scala @@ -26,7 +26,7 @@ import scala.concurrent.duration.Duration import scala.util.{ Failure, Success, Try } import scala.util.control.{ ControlThrowable, NonFatal } -import com.typesafe.config.{ Config, ConfigFactory } +import com.typesafe.config.{ Config, ConfigFactory, ConfigRenderOptions } import org.apache.pekko import pekko.ConfigurationException @@ -489,7 +489,8 @@ object ActorSystem { /** * Returns the String representation of the Config that this Settings is backed by */ - override def toString: String = config.root.render + override def toString: String = + config.root.render(ConfigRenderOptions.defaults().setShowEnvVariableValues(false)) } diff --git a/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala b/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala index 791cba0ff09..befe3dc32b1 100644 --- a/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala +++ b/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala @@ -14,9 +14,10 @@ package org.apache.pekko.dispatch import java.util.concurrent.{ ConcurrentHashMap, ThreadFactory } -import scala.concurrent.ExecutionContext import scala.annotation.{ nowarn, tailrec } -import com.typesafe.config.{ Config, ConfigFactory, ConfigValueType } +import scala.concurrent.ExecutionContext + +import com.typesafe.config.{ Config, ConfigFactory, ConfigRenderOptions, ConfigValueType } import org.apache.pekko import pekko.ConfigurationException import pekko.actor.{ ActorSystem, DynamicAccess, Scheduler } @@ -259,7 +260,8 @@ class Dispatchers @InternalApi private[pekko] ( */ private def configuratorFrom(cfg: Config): MessageDispatcherConfigurator = { if (!cfg.hasPath("id")) - throw new ConfigurationException("Missing dispatcher 'id' property in config: " + cfg.root.render) + throw new ConfigurationException("Missing dispatcher 'id' property in config: " + + cfg.root.render(ConfigRenderOptions.defaults().setShowEnvVariableValues(false))) cfg.getString("type") match { case "Dispatcher" => new DispatcherConfigurator(cfg, prerequisites) From d7d8ee485443630de889537a9cbe450b529ad1ed Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Thu, 2 Nov 2023 21:23:51 +0100 Subject: [PATCH 2/5] redact username when logging configs --- .../apache/pekko/actor/ActorSystemSpec.scala | 16 ++++++++++++ .../pekko/actor/typed/ActorSystem.scala | 8 +++--- .../org/apache/pekko/actor/ActorSystem.scala | 5 ++-- .../apache/pekko/dispatch/Dispatchers.scala | 4 +-- .../scala/org/apache/pekko/util/Helpers.scala | 26 ++++++++++++++----- 5 files changed, 42 insertions(+), 17 deletions(-) diff --git a/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala b/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala index a5c25b9ee98..ada7eb3600e 100644 --- a/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala +++ b/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala @@ -392,6 +392,22 @@ class ActorSystemSpec extends PekkoSpec(ActorSystemSpec.config) with ImplicitSen } } finally shutdown(sys) } + "not include env variables in toString" in { + // Actor System toString is output to logs and we don't want env variable values appearing in logs + // "pekko.test.env.var.home = ${HOME}\n" + val system = + ActorSystem( + "config-test-system", + ConfigFactory + .parseString( + """pekko.test.java.property.home = "${user.home}"""") + .withFallback(PekkoSpec.testConf)) + val debugText = system.settings.toString + val username = System.getProperty("user.name") + (debugText should not).include(username) + debugText should include("") + shutdown(system) + } } } diff --git a/actor-typed/src/main/scala/org/apache/pekko/actor/typed/ActorSystem.scala b/actor-typed/src/main/scala/org/apache/pekko/actor/typed/ActorSystem.scala index 7f423668d4d..d60d91077ee 100644 --- a/actor-typed/src/main/scala/org/apache/pekko/actor/typed/ActorSystem.scala +++ b/actor-typed/src/main/scala/org/apache/pekko/actor/typed/ActorSystem.scala @@ -17,8 +17,7 @@ import java.util.concurrent.{ CompletionStage, ThreadFactory } import scala.concurrent.{ ExecutionContextExecutor, Future } -import com.typesafe.config.{ Config, ConfigFactory, ConfigRenderOptions } - +import com.typesafe.config.{ Config, ConfigFactory } import org.slf4j.Logger import org.apache.pekko import pekko.{ actor => classic, Done } @@ -29,7 +28,7 @@ import pekko.actor.typed.internal.{ EventStreamExtension, InternalRecipientRef } import pekko.actor.typed.internal.adapter.{ ActorSystemAdapter, GuardianStartupBehavior, PropsAdapter } import pekko.actor.typed.receptionist.Receptionist import pekko.annotation.DoNotInherit -import pekko.util.Helpers.Requiring +import pekko.util.Helpers.{ ConfigOps, Requiring } /** * An ActorSystem is home to a hierarchy of Actors. It is created using @@ -324,8 +323,7 @@ final class Settings(val config: Config, val classicSettings: classic.ActorSyste /** * Returns the String representation of the Config that this Settings is backed by */ - override def toString: String = - config.root.render(ConfigRenderOptions.defaults().setShowEnvVariableValues(false)) + override def toString: String = config.renderWithRedactions() private val typedConfig = config.getConfig("pekko.actor.typed") diff --git a/actor/src/main/scala/org/apache/pekko/actor/ActorSystem.scala b/actor/src/main/scala/org/apache/pekko/actor/ActorSystem.scala index df1ad2efc6a..b91961d07d5 100644 --- a/actor/src/main/scala/org/apache/pekko/actor/ActorSystem.scala +++ b/actor/src/main/scala/org/apache/pekko/actor/ActorSystem.scala @@ -26,7 +26,7 @@ import scala.concurrent.duration.Duration import scala.util.{ Failure, Success, Try } import scala.util.control.{ ControlThrowable, NonFatal } -import com.typesafe.config.{ Config, ConfigFactory, ConfigRenderOptions } +import com.typesafe.config.{ Config, ConfigFactory } import org.apache.pekko import pekko.ConfigurationException @@ -489,8 +489,7 @@ object ActorSystem { /** * Returns the String representation of the Config that this Settings is backed by */ - override def toString: String = - config.root.render(ConfigRenderOptions.defaults().setShowEnvVariableValues(false)) + override def toString: String = config.renderWithRedactions() } diff --git a/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala b/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala index befe3dc32b1..06d7c2f5106 100644 --- a/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala +++ b/actor/src/main/scala/org/apache/pekko/dispatch/Dispatchers.scala @@ -17,7 +17,7 @@ import java.util.concurrent.{ ConcurrentHashMap, ThreadFactory } import scala.annotation.{ nowarn, tailrec } import scala.concurrent.ExecutionContext -import com.typesafe.config.{ Config, ConfigFactory, ConfigRenderOptions, ConfigValueType } +import com.typesafe.config.{ Config, ConfigFactory, ConfigValueType } import org.apache.pekko import pekko.ConfigurationException import pekko.actor.{ ActorSystem, DynamicAccess, Scheduler } @@ -261,7 +261,7 @@ class Dispatchers @InternalApi private[pekko] ( private def configuratorFrom(cfg: Config): MessageDispatcherConfigurator = { if (!cfg.hasPath("id")) throw new ConfigurationException("Missing dispatcher 'id' property in config: " + - cfg.root.render(ConfigRenderOptions.defaults().setShowEnvVariableValues(false))) + cfg.renderWithRedactions()) cfg.getString("type") match { case "Dispatcher" => new DispatcherConfigurator(cfg, prerequisites) diff --git a/actor/src/main/scala/org/apache/pekko/util/Helpers.scala b/actor/src/main/scala/org/apache/pekko/util/Helpers.scala index 2d9b0ef1785..6b00bd58ed9 100644 --- a/actor/src/main/scala/org/apache/pekko/util/Helpers.scala +++ b/actor/src/main/scala/org/apache/pekko/util/Helpers.scala @@ -28,16 +28,12 @@ package org.apache.pekko.util import java.time.{ Instant, LocalDateTime, ZoneId } import java.time.format.DateTimeFormatter -import java.util.Comparator -import java.util.Locale +import java.util.{ Comparator, Locale } import java.util.concurrent.TimeUnit import java.util.regex.Pattern - import scala.annotation.tailrec -import scala.concurrent.duration.Duration -import scala.concurrent.duration.FiniteDuration - -import com.typesafe.config.Config +import scala.concurrent.duration.{ Duration, FiniteDuration } +import com.typesafe.config.{ Config, ConfigRenderOptions } object Helpers { @@ -179,6 +175,22 @@ object Helpers { def getNanosDuration(path: String): FiniteDuration = getDuration(path, TimeUnit.NANOSECONDS) + /** + * Used to redact sensitive information in config data when we are logging it + * or adding it to exception messages. + * + * This includes redacting environment variable values and the username associated with the running process. + * + * @return redacted version of the configuration text + * @see https://github.com/apache/incubator-pekko/pull/771 + * @since 1.0.2 + */ + def renderWithRedactions(): String = { + val username = System.getProperty("user.name") + val configText = config.root.render(ConfigRenderOptions.defaults().setShowEnvVariableValues(false)) + configText.replace(username, "") + } + private def getDuration(path: String, unit: TimeUnit): FiniteDuration = Duration(config.getDuration(path, unit), unit) } From 195e01a93f83ca8a5d384e897f9b5320f832e4e9 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Thu, 2 Nov 2023 21:40:59 +0100 Subject: [PATCH 3/5] Update ActorSystemSpec.scala --- .../scala/org/apache/pekko/actor/ActorSystemSpec.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala b/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala index ada7eb3600e..88ce8b05710 100644 --- a/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala +++ b/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala @@ -392,19 +392,19 @@ class ActorSystemSpec extends PekkoSpec(ActorSystemSpec.config) with ImplicitSen } } finally shutdown(sys) } - "not include env variables in toString" in { + "not include username in toString" in { // Actor System toString is output to logs and we don't want env variable values appearing in logs - // "pekko.test.env.var.home = ${HOME}\n" val system = ActorSystem( "config-test-system", ConfigFactory - .parseString( - """pekko.test.java.property.home = "${user.home}"""") + .parseString("""pekko.test.java.property.home = "${user.home}"""") .withFallback(PekkoSpec.testConf)) val debugText = system.settings.toString val username = System.getProperty("user.name") + val userHome = System.getProperty("user.home") (debugText should not).include(username) + (debugText should not).include(userHome) debugText should include("") shutdown(system) } From cbd7cec62790e5c0cb3e3cc8908710672ebb7a4d Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Fri, 3 Nov 2023 11:02:43 +0100 Subject: [PATCH 4/5] add test scalafmt --- .../pekko/actor/typed/ActorSystemSpec.scala | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) create mode 100644 actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/ActorSystemSpec.scala diff --git a/actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/ActorSystemSpec.scala b/actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/ActorSystemSpec.scala new file mode 100644 index 00000000000..cfd411d59ba --- /dev/null +++ b/actor-typed-tests/src/test/scala/org/apache/pekko/actor/typed/ActorSystemSpec.scala @@ -0,0 +1,48 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.pekko.actor.typed + +import com.typesafe.config.ConfigFactory +import org.apache.pekko +import pekko.actor.typed.scaladsl.Behaviors +import pekko.testkit.PekkoSpec + +import scala.annotation.nowarn + +@nowarn("msg=possible missing interpolator") +class ActorSystemSpec extends PekkoSpec { + "ActorSystem" should { + "not include username in toString" in { + // Actor System toString is output to logs and we don't want env variable values appearing in logs + val system = ActorSystem(Behaviors.empty[String], "config-test-system", + ConfigFactory + .parseString("""pekko.test.java.property.home = "${user.home}"""") + .withFallback(PekkoSpec.testConf)) + try { + val debugText = system.settings.toString + val username = System.getProperty("user.name") + val userHome = System.getProperty("user.home") + (debugText should not).include(username) + (debugText should not).include(userHome) + debugText should include("") + } finally { + system.terminate() + } + } + } +} From 225f648eb673df87b4f7bd5f33649a53f6adb876 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Mon, 13 Nov 2023 20:41:33 +0100 Subject: [PATCH 5/5] try/finally --- .../org/apache/pekko/actor/ActorSystemSpec.scala | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala b/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala index 88ce8b05710..c8af272fa9a 100644 --- a/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala +++ b/actor-tests/src/test/scala/org/apache/pekko/actor/ActorSystemSpec.scala @@ -400,13 +400,14 @@ class ActorSystemSpec extends PekkoSpec(ActorSystemSpec.config) with ImplicitSen ConfigFactory .parseString("""pekko.test.java.property.home = "${user.home}"""") .withFallback(PekkoSpec.testConf)) - val debugText = system.settings.toString - val username = System.getProperty("user.name") - val userHome = System.getProperty("user.home") - (debugText should not).include(username) - (debugText should not).include(userHome) - debugText should include("") - shutdown(system) + try { + val debugText = system.settings.toString + val username = System.getProperty("user.name") + val userHome = System.getProperty("user.home") + (debugText should not).include(username) + (debugText should not).include(userHome) + debugText should include("") + } finally shutdown(system) } }