Properly handle pgp keychains generated by Scala CLI

modules/build/src/main/scala/scala/build/internal/util/WarningMessages.scala

@@ -19,4 +19,14 @@ object WarningMessages {
 
   def experimentalConfigKeyUsed(name: String): String =
     experimentalFeatureUsed(s"The '$name' configuration key")
+
+  def rawValueNotWrittenToPublishFile(
+    rawValue: String,
+    valueName: String,
+    directiveName: String
+  ): String =
+    s"""The value of $valueName ${Console.BOLD}will not${Console.RESET} be written to a potentially public file!
+       |Provide it as an option to the publish subcommand with:
+       | $directiveName value:$rawValue
+       |""".stripMargin
 }

modules/cli/src/main/scala/scala/cli/commands/config/Config.scala

@@ -51,10 +51,15 @@ object Config extends ScalaCommand[ConfigOptions] {
       args.all match {
         case Seq() =>
           if (options.createPgpKey) {
-            val coursierCache       = options.coursier.coursierCache(logger.coursierLogger(""))
-            val secKeyEntry         = Keys.pgpSecretKey
-            val secKeyPasswordEntry = Keys.pgpSecretKeyPassword
-            val pubKeyEntry         = Keys.pgpPublicKey
+            if (options.pgpPassword.isEmpty) {
+              logger.error(
+                s"--pgp-password not specified, use 'none' to create an unprotected keychain or 'random' to generate a password"
+              )
+              sys.exit(1)
+            }
+            val coursierCache = options.coursier.coursierCache(logger.coursierLogger(""))
+            val secKeyEntry   = Keys.pgpSecretKey
+            val pubKeyEntry   = Keys.pgpPublicKey
 
             val mail = options.email
               .filter(_.trim.nonEmpty)
@@ -64,17 +69,23 @@ object Config extends ScalaCommand[ConfigOptions] {
                   .orExit(logger)
               }
               .getOrElse {
-                System.err.println(
-                  s"Error: --email ... not specified, and ${Keys.userEmail.fullName} not set (either is required to generate a PGP key)"
+                logger.error(
+                  s"--email ... not specified, and ${Keys.userEmail.fullName} not set (either is required to generate a PGP key)"
                 )
                 sys.exit(1)
               }
 
-            val password = ThrowawayPgpSecret.pgpPassPhrase()
+            val passwordOpt = if (options.pgpPassword.contains("none"))
+              None
+            else if (options.pgpPassword.contains("random"))
+              Some(ThrowawayPgpSecret.pgpPassPhrase())
+            else
+              options.pgpPassword.map(scala.cli.signing.shared.Secret.apply)
+
             val (pgpPublic, pgpSecret0) =
               ThrowawayPgpSecret.pgpSecret(
                 mail,
-                Some(password),
+                passwordOpt,
                 logger,
                 coursierCache,
                 () =>
@@ -88,11 +99,20 @@ object Config extends ScalaCommand[ConfigOptions] {
             val pgpSecretBase64 = pgpSecret0.map(Base64.getEncoder.encodeToString)
 
             db.set(secKeyEntry, PasswordOption.Value(pgpSecretBase64.toConfig))
-            db.set(secKeyPasswordEntry, PasswordOption.Value(password.toConfig))
             db.set(pubKeyEntry, PasswordOption.Value(pgpPublic.toConfig))
             db.save(directories.dbPath.toNIO)
               .wrapConfigException
               .orExit(logger)
+
+            logger.message("PGP keychains written to config")
+            if (options.pgpPassword.contains("random"))
+              passwordOpt.foreach { password =>
+                println(
+                  s"""Password: ${password.value}
+                     |Don't lose it!
+                     |""".stripMargin
+                )
+              }
           }
           else {
             System.err.println("No argument passed")
@@ -122,7 +142,7 @@ object Config extends ScalaCommand[ConfigOptions] {
                   valueOpt match {
                     case Some(value) =>
                       for (v <- value)
-                        if (options.password && entry.isPasswordOption)
+                        if (options.passwordValue && entry.isPasswordOption)
                           PasswordOption.parse(v) match {
                             case Left(err) =>
                               System.err.println(err)

modules/cli/src/main/scala/scala/cli/commands/config/ConfigOptions.scala

@@ -27,20 +27,26 @@ final case class ConfigOptions(
   @Tag(tags.inShortHelp)
     dump: Boolean = false,
   @Group(HelpGroup.Config.toString)
-  @HelpMessage("Create PGP key in config")
+  @HelpMessage("Create PGP keychain in config")
+  @Tag(tags.inShortHelp)
   @Tag(tags.experimental)
     createPgpKey: Boolean = false,
   @Group(HelpGroup.Config.toString)
-  @HelpMessage("Email to use to create PGP key in config")
+  @HelpMessage("A password used to encode the private PGP keychain")
   @Tag(tags.experimental)
-    email: Option[String] = None,
+  @ValueDescription("YOUR_PASSWORD|random|none")
+  @ExtraName("passphrase")
+  pgpPassword: Option[String] = None,
   @Group(HelpGroup.Config.toString)
-  @HelpMessage("If the entry is a password, print the password value rather than how to get the password")
-  @Tag(tags.restricted)
-  @Tag(tags.inShortHelp)
-    password: Boolean = false,
+  @HelpMessage("Email used to create the PGP keychains in config")
+  @Tag(tags.experimental)
+    email: Option[String] = None,
   @Group(HelpGroup.Config.toString)
-  @HelpMessage("If the entry is a password, save the password value rather than how to get the password")
+  @HelpMessage(
+    """When accessing config's content print the password value rather than how to get the password
+      |When saving an entry in config save the password value rather than how to get the password
+      |e.g. print/save the value of environment variable ENV_VAR rather than "env:ENV_VAR"
+      |""".stripMargin)
   @Tag(tags.restricted)
   @Tag(tags.inShortHelp)
     passwordValue: Boolean = false,

modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala

@@ -11,6 +11,7 @@ import scala.build.EitherCps.{either, value}
 import scala.build.Logger
 import scala.build.Ops.*
 import scala.build.errors.{BuildException, CompositeBuildException, MalformedCliInputError}
+import scala.build.internal.util.WarningMessages
 import scala.build.options.publish.ConfigPasswordOption
 import scala.build.options.publish.ConfigPasswordOption.*
 import scala.build.options.PublishOptions as BPublishOptions
@@ -321,8 +322,6 @@ final case class PgpSecretKeyCheck(
 
           if (keysFromConfig.publicKeyOpt.isEmpty)
             logger.message("  warning: no PGP public key found in config")
-          if (keysFromConfig.secretKeyPasswordOpt.isEmpty)
-            logger.message("  warning: no PGP secret key password found in config")
 
           (keysFromConfig, true)
         }
@@ -392,34 +391,50 @@ final case class PgpSecretKeyCheck(
                 scala.util.Try(path.relativeTo(os.pwd))
                   .map(p => s"file:${p.toString}")
                   .getOrElse(optionValue)
-
               }
               else optionValue
             case ConfigOption(fullName) => s"config:$fullName"
           }
 
-        val passwordDirectives = getDirectiveValue(setupKeys.secretKeyPasswordOpt).map {
-          "publish.secretKeyPassword" -> _
-        }.toSeq
-
-        val secretKeyDirValue = getDirectiveValue(setupKeys.secretKeyOpt)
+        val rawValueRegex = "^value:(.*)".r
+
+        // Prevent potential leakage of a secret value
+        val passwordDirectives = getDirectiveValue(setupKeys.secretKeyPasswordOpt)
+          .flatMap {
+            case rawValueRegex(rawValue) =>
+              logger.diagnostic(
+                WarningMessages.rawValueNotWrittenToPublishFile(
+                  rawValue,
+                  "PGP password",
+                  "--secret-key-password"
+                )
+              )
+              None
+            case secretOption => Some("publish.secretKeyPassword" -> secretOption)
+          }
+          .toSeq
+
+        // Prevent potential leakage of a secret value
+        val secretKeyDirValue = getDirectiveValue(setupKeys.secretKeyOpt).flatMap {
+          case rawValueRegex(rawValue) =>
+            logger.diagnostic(
+              WarningMessages.rawValueNotWrittenToPublishFile(
+                rawValue,
+                "PGP secret key",
+                "--secret-key"
+              )
+            )
+            None
+          case secretOption => Some(secretOption)
+        }
 
+        // This is safe to be publicly available
         val publicKeyDirective = getDirectiveValue(setupKeys.publicKeyOpt).map {
           "publish.publicKey" -> _
         }.toSeq
 
         val extraDirectives = passwordDirectives ++ publicKeyDirective
 
-        if (passwordDirectives.exists(_._2.startsWith("value:")))
-          logger.diagnostic(
-            "The secret value of PGP private key password will be written to a potentially public file!"
-          )
-
-        if (secretKeyDirValue.exists(_.startsWith("value:")))
-          logger.diagnostic(
-            "The secret value of PGP private key will be written to a potentially public file!"
-          )
-
         OptionCheck.DefaultValue(
           () => uploadKey(publicKeyOpt).map(_ => secretKeyDirValue),
           extraDirectives,

modules/integration/src/test/scala/scala/cli/integration/ConfigTests.scala

@@ -67,7 +67,7 @@ class ConfigTests extends ScalaCliSuite {
         res.out.trim()
       }
       def readDecoded(env: Map[String, String] = Map.empty): String = {
-        val res = os.proc(TestUtil.cli, "--power", "config", key, "--password")
+        val res = os.proc(TestUtil.cli, "--power", "config", key, "--password-value")
           .call(cwd = root, env = configEnv ++ env)
         res.out.trim()
       }
@@ -166,10 +166,12 @@ class ConfigTests extends ScalaCliSuite {
   }
 
   if (!TestUtil.isCI || !Properties.isWin)
-    test("Create a default PGP key") {
-      createDefaultPgpKeyTest()
-    }
-  def createDefaultPgpKeyTest(): Unit = {
+    for (pgpPasswordOption <- List("none", "random", "MY_CHOSEN_PASSWORD"))
+      test(s"Create a default PGP key, password: $pgpPasswordOption") {
+        createDefaultPgpKeyTest(pgpPasswordOption)
+      }
+
+  def createDefaultPgpKeyTest(pgpPasswordOption: String): Unit = {
     TestInputs().fromRoot { root =>
       val configFile = {
         val dir = root / "config"
@@ -179,22 +181,57 @@ class ConfigTests extends ScalaCliSuite {
       val extraEnv = Map(
         "SCALA_CLI_CONFIG" -> configFile.toString
       )
-      val checkRes = os.proc(TestUtil.cli, "--power", "config", "--create-pgp-key")
+      val checkPassword = os.proc(TestUtil.cli, "--power", "config", "--create-pgp-key")
         .call(cwd = root, env = extraEnv, check = false, mergeErrIntoOut = true)
-      expect(checkRes.exitCode != 0)
-      expect(checkRes.out.text().contains("--email"))
-      os.proc(TestUtil.cli, "--power", "config", "--create-pgp-key", "--email", "alex@alex.me")
-        .call(cwd = root, env = extraEnv, stdin = os.Inherit, stdout = os.Inherit)
+      expect(checkPassword.exitCode != 0)
+      expect(checkPassword.out.text().contains("--pgp-password"))
+
+      val checkEmail = os.proc(
+        TestUtil.cli,
+        "--power",
+        "config",
+        "--create-pgp-key",
+        "--pgp-password",
+        pgpPasswordOption
+      )
+        .call(cwd = root, env = extraEnv, check = false, mergeErrIntoOut = true)
+      expect(checkEmail.exitCode != 0)
+      expect(checkEmail.out.text().contains("--email"))
+
+      val pgpCreated = os.proc(
+        TestUtil.cli,
+        "--power",
+        "config",
+        "--create-pgp-key",
+        "--email",
+        "alex@alex.me",
+        "--pgp-password",
+        pgpPasswordOption
+      )
+        .call(cwd = root, env = extraEnv, mergeErrIntoOut = true)
+
+      val pgpPasswordOpt: Option[String] = pgpCreated.out.text()
+        .linesIterator
+        .toSeq
+        .find(_.startsWith("Password"))
+        .map(_.stripPrefix("Password:").trim())
+
+      if (pgpPasswordOption != "random")
+        expect(pgpPasswordOpt.isEmpty)
+      else
+        expect(pgpPasswordOpt.isDefined)
+
+      val passwordInConfig = os.proc(TestUtil.cli, "--power", "config", "pgp.secret-key-password")
+        .call(cwd = root, env = extraEnv, stderr = os.Pipe)
+      expect(passwordInConfig.out.text().isEmpty())
 
-      val password = os.proc(TestUtil.cli, "--power", "config", "pgp.secret-key-password")
-        .call(cwd = root, env = extraEnv)
-        .out.trim()
       val secretKey = os.proc(TestUtil.cli, "--power", "config", "pgp.secret-key")
-        .call(cwd = root, env = extraEnv)
-        .out.trim()
-      val rawPublicKey = os.proc(TestUtil.cli, "--power", "config", "pgp.public-key", "--password")
-        .call(cwd = root, env = extraEnv)
+        .call(cwd = root, env = extraEnv, stderr = os.Pipe)
         .out.trim()
+      val rawPublicKey =
+        os.proc(TestUtil.cli, "--power", "config", "pgp.public-key", "--password-value")
+          .call(cwd = root, env = extraEnv, stderr = os.Pipe)
+          .out.trim()
 
       val tmpFile    = root / "test-file"
       val tmpFileAsc = root / "test-file.asc"
@@ -204,23 +241,37 @@ class ConfigTests extends ScalaCliSuite {
       def maybeEscape(arg: String): String =
         if (Properties.isWin) q + arg + q
         else arg
-      os.proc(
-        TestUtil.cli,
-        "--power",
-        "pgp",
-        "sign",
-        "--password",
-        maybeEscape(password),
-        "--secret-key",
-        maybeEscape(secretKey),
-        tmpFile
-      )
-        .call(cwd = root, stdin = os.Inherit, stdout = os.Inherit, env = extraEnv)
+      val signProcess = if (pgpPasswordOption != "none")
+        os.proc(
+          TestUtil.cli,
+          "--power",
+          "pgp",
+          "sign",
+          "--password",
+          s"value:${maybeEscape(pgpPasswordOpt.getOrElse("MY_CHOSEN_PASSWORD"))}",
+          "--secret-key",
+          maybeEscape(secretKey),
+          tmpFile
+        )
+      else
+        os.proc(
+          TestUtil.cli,
+          "--power",
+          "pgp",
+          "sign",
+          "--secret-key",
+          maybeEscape(secretKey),
+          tmpFile
+        )
+      signProcess.call(cwd = root, stdin = os.Inherit, stdout = os.Inherit, env = extraEnv)
 
       val pubKeyFile = root / "key.pub"
       os.write(pubKeyFile, rawPublicKey)
-      os.proc(TestUtil.cli, "--power", "pgp", "verify", "--key", pubKeyFile, tmpFileAsc)
-        .call(cwd = root, stdin = os.Inherit, stdout = os.Inherit, env = extraEnv)
+      val verifyResult =
+        os.proc(TestUtil.cli, "--power", "pgp", "verify", "--key", pubKeyFile, tmpFileAsc)
+          .call(cwd = root, env = extraEnv, mergeErrIntoOut = true)
+
+      expect(verifyResult.out.text().contains("valid signature"))
     }
   }