WX-1385 Remove SAS tokens from TES input log printouts (#7358)

broadinstitute · Feb 14, 2024 · 9b1ac0f · 9b1ac0f
1 parent 1043c37
commit 9b1ac0f
Show file tree

Hide file tree

Showing 3 changed files with 45 additions and 2 deletions.
diff --git a/common/src/main/scala/common/util/UriUtil.scala b/common/src/main/scala/common/util/UriUtil.scala
@@ -87,8 +87,13 @@ object UriUtil {
       "signature"
     )
 
+  private val SensitiveKeys =
+    List(
+      "sig"
+    )
+
   private def isSensitiveKey(name: String): Boolean = {
     val lower = name.toLowerCase
-    SensitiveKeyParts.exists(lower.contains(_))
+    SensitiveKeyParts.exists(lower.contains(_)) || SensitiveKeys.exists(lower.equals(_))
   }
 }
diff --git a/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala b/supportedBackends/tes/src/main/scala/cromwell/backend/impl/tes/TesTask.scala
@@ -367,7 +367,15 @@ final case class Input(name: Option[String],
                        path: String,
                        `type`: Option[String],
                        content: Option[String]
-)
+) {
+  override def toString: String = {
+    import common.util.StringUtil.EnhancedString
+
+    // Mask SAS token signature in query
+    this.getClass.getName + Seq(name, description, url.map(_.maskSensitiveUri), path, `type`, content)
+      .mkString("(", ",", ")")
+  }
+}
 
 final case class Output(name: Option[String],
                         description: Option[String],

diff --git a/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala b/supportedBackends/tes/src/test/scala/cromwell/backend/impl/tes/TesTaskSpec.scala
@@ -217,4 +217,34 @@ class TesTaskSpec extends AnyFlatSpec with CromwellTimeoutSpec with Matchers wit
       "parent_workflow_id" -> Option(subWorkflowId.toString)
     )
   }
+
+  it should "not leak secrets when printing file paths" in {
+    val input = Input(
+      Option("asdf"),
+      Option("asdf"),
+      url = Option(
+        "https://lz304a1e79fd7359e5327eda.blob.core.windows.net/sc-705b830a-d699-478e-9da6-49661b326e77" +
+          "?sv=2021-12-02&spr=https&st=2023-12-13T20%3A27%3A55Z&se=2023-12-14T04%3A42%3A55Z&sr=c&sp=racwdlt&sig=SECRET&rscd=foo"
+      ),
+      "asdf",
+      Option("asdf"),
+      Option("asdf")
+    )
+
+    input.toString shouldBe "cromwell.backend.impl.tes.Input(Some(asdf),Some(asdf),Some(https://lz304a1e79fd7359e5327eda.blob.core.windows.net/sc-705b830a-d699-478e-9da6-49661b326e77" +
+      "?sv=2021-12-02&spr=https&st=2023-12-13T20:27:55Z&se=2023-12-14T04:42:55Z&sr=c&sp=racwdlt&sig=masked&rscd=foo),asdf,Some(asdf),Some(asdf))"
+  }
+
+  it should "not crash if the URL is missing" in {
+    val input = Input(
+      Option("asdf"),
+      Option("asdf"),
+      url = None,
+      "asdf",
+      Option("asdf"),
+      Option("asdf")
+    )
+
+    input.toString shouldBe "cromwell.backend.impl.tes.Input(Some(asdf),Some(asdf),None,asdf,Some(asdf),Some(asdf))"
+  }
 }