From 422eb2285ca022944a56baff1d0c099f7062f0a8 Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Thu, 2 Mar 2023 08:21:09 +0100 Subject: [PATCH 1/9] Prevent NPE when a pgp key option is not passed --- .../scala/cli/commands/publish/Publish.scala | 194 ++++++++------ .../errors/MissingPublishOptionError.scala | 5 +- .../integration/PublishTestDefinitions.scala | 250 ++++++++++++++++-- .../cli/integration/PublishTestsDefault.scala | 7 +- 4 files changed, 345 insertions(+), 111 deletions(-) diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala index d6222ce6c3..2f76d9de5e 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala @@ -53,7 +53,7 @@ import scala.cli.commands.shared.{ } import scala.cli.commands.util.{BuildCommandHelpers, ScalaCliSttpBackend} import scala.cli.commands.{ScalaCommand, SpecificationLevel, WatchUtil} -import scala.cli.config.{ConfigDb, Keys, PublishCredentials} +import scala.cli.config.{ConfigDb, Keys, PasswordOption, PublishCredentials} import scala.cli.errors.{ FailedToSignFileError, MalformedChecksumsError, @@ -844,97 +844,117 @@ object Publish extends ScalaCommand[PublishOptions] with BuildCommandHelpers { } } - val signerOpt = publishOptions.contextual(isCi).signer.orElse { - if (repoParams.supportsSig) - if (publishOptions.contextual(isCi).secretKey.isDefined) Some(PSigner.BouncyCastle) - else if (publishOptions.contextual(isCi).gpgSignatureId.isDefined) Some(PSigner.Gpg) - else if (repoParams.shouldSign) Some(PSigner.BouncyCastle) - else None - else None + def getBouncyCastleSigner( + secretKey: PasswordOption, + secretKeyPasswordOpt: Option[PasswordOption] + ) = { + val getLauncher: Supplier[Array[String]] = { () => + val buildOptions = builds.headOption.map(_.options) + val archiveCache = buildOptions.map(_.archiveCache) + .getOrElse(ArchiveCache()) + val fileCache = buildOptions.map(_.finalCache).getOrElse(FileCache()) + PgpExternalCommand.launcher( + fileCache, + archiveCache, + logger, + () => builds.head.options.javaHome().value.javaCommand, + publishOptions.signingCli + ) match { + case Left(e) => throw new Exception(e) + case Right(binaryCommand) => binaryCommand.toArray + } + } + + if (forceSigningBinary) + (new scala.cli.internal.BouncycastleSignerMakerSubst).get( + secretKeyPasswordOpt.fold(null)(_.toCliSigning), + secretKey.toCliSigning, + getLauncher, + logger + ) + else + (new BouncycastleSignerMaker).get( + secretKeyPasswordOpt.fold(null)(_.toCliSigning), + secretKey.toCliSigning, + getLauncher, + logger + ) + } + + val signerKind: PSigner = publishOptions.contextual(isCi).signer.getOrElse { + if (!repoParams.supportsSig) + PSigner.Nop + else if (publishOptions.contextual(isCi).gpgSignatureId.isDefined) + PSigner.Gpg + else if (repoParams.shouldSign) + PSigner.BouncyCastle + else + PSigner.Nop + } + + def getSecretKeyPasswordOpt: Option[PasswordOption] = { + val maybeSecretKeyPass = if (publishOptions.contextual(isCi).secretKeyPassword.isDefined) + for { + secretKeyPassConfigOpt <- publishOptions.contextual(isCi).secretKeyPassword + secretKeyPass <- secretKeyPassConfigOpt.get(configDb()).toOption + } yield secretKeyPass + else + for { + secretKeyPassOpt <- configDb().get(Keys.pgpSecretKeyPassword).toOption + secretKeyPass <- secretKeyPassOpt + } yield secretKeyPass + + maybeSecretKeyPass } - val signer: Signer = signerOpt match { - case Some(PSigner.Gpg) => - publishOptions.contextual(isCi).gpgSignatureId match { - case Some(gpgSignatureId) => - GpgSigner( - GpgSigner.Key.Id(gpgSignatureId), - extraOptions = publishOptions.contextual(isCi).gpgOptions + + val signer: Either[BuildException, Signer] = signerKind match { + // user specified --signer=gpg or --gpgKey=... + case PSigner.Gpg => + publishOptions.contextual(isCi).gpgSignatureId.map { gpgSignatureId => + GpgSigner( + GpgSigner.Key.Id(gpgSignatureId), + extraOptions = publishOptions.contextual(isCi).gpgOptions + ) + }.toRight(new MissingPublishOptionError( + "ID of the GPG key", + "--gpgKey", + directiveName = "" + )) + + // user specified --signer=bc or --secret-key=... or target repository requires signing + // --secret-key-password is possibly specified (not mandatory) + case PSigner.Nop | PSigner.BouncyCastle + if publishOptions.contextual(isCi).secretKey.isDefined => + val secretKeyConfigOpt = publishOptions.contextual(isCi).secretKey.get + for { + secretKey <- secretKeyConfigOpt.get(configDb()) + } yield getBouncyCastleSigner(secretKey, getSecretKeyPasswordOpt) + + // user specified --signer=bc or target repository requires signing + // --secret-key-password is possibly specified (not mandatory) + case PSigner.BouncyCastle => + val shouldSignMsg = + if (repoParams.shouldSign) "signing is required for chosen repository" else "" + for { + secretKeyOpt <- configDb().get(Keys.pgpSecretKey).wrapConfigException + secretKey <- secretKeyOpt.toRight( + new MissingPublishOptionError( + "secret key", + "--secret-key", + directiveName = "", + configKeys = Seq("pgp.secret-key"), + extraMessage = shouldSignMsg ) - case None => NopSigner - } - case Some(PSigner.BouncyCastle) => - val getLauncher: Supplier[Array[String]] = { () => - val buildOptions = builds.headOption.map(_.options) - val archiveCache = buildOptions.map(_.archiveCache) - .getOrElse(ArchiveCache()) - val fileCache = buildOptions.map(_.finalCache).getOrElse(FileCache()) - PgpExternalCommand.launcher( - fileCache, - archiveCache, - logger, - () => builds.head.options.javaHome().value.javaCommand, - publishOptions.signingCli - ) match { - case Left(e) => throw new Exception(e) - case Right(binaryCommand) => binaryCommand.toArray - } - } - val secretKeyDetailsOpt = publishOptions.contextual(isCi).secretKey match { - case Some(secretKey0) => - val secretKey = secretKey0.get(configDb()).orExit(logger).toCliSigning - val secretKeyPassword = publishOptions - .contextual(isCi) - .secretKeyPassword - .orNull - .get(configDb()) - .orExit(logger) - .toCliSigning - Some((secretKey, secretKeyPassword)) - case None => - configDb().get(Keys.pgpSecretKey).wrapConfigException.orExit(logger) match { - case Some(secretKey) => - val secretKeyPassword = - configDb().get(Keys.pgpSecretKeyPassword).wrapConfigException - .flatMap { - case None => - Left(new MissingConfigEntryError(Keys.pgpSecretKeyPassword.fullName)) - case Some(p) => Right(p) - } - .orExit(logger) - Some((secretKey.toCliSigning, secretKeyPassword.toCliSigning)) - case None => - None - } - } - secretKeyDetailsOpt match { - case Some((secretKey, secretKeyPassword)) => - if (forceSigningBinary) - (new scala.cli.internal.BouncycastleSignerMakerSubst).get( - secretKeyPassword, - secretKey, - getLauncher, - logger - ) - else - (new BouncycastleSignerMaker).get( - secretKeyPassword, - secretKey, - getLauncher, - logger - ) - case None => - if (repoParams.shouldSign) - logger.diagnostic( - "PGP signatures are disabled, while these are recommended for this repository." - ) - NopSigner - } - case Some(PSigner.Nop) => NopSigner - case None => NopSigner + ) + } yield getBouncyCastleSigner(secretKey, getSecretKeyPasswordOpt) + case _ => + logger.diagnostic("Not signing files as it's not needed nor has it been specified") + Right(NopSigner) } + val signerLogger = new InteractiveSignerLogger(new OutputStreamWriter(System.err), verbosity = 1) - val signRes = signer.signatures( + val signRes = value(signer).signatures( fileSet0, now, ChecksumType.all.map(_.extension).toSet, diff --git a/modules/cli/src/main/scala/scala/cli/errors/MissingPublishOptionError.scala b/modules/cli/src/main/scala/scala/cli/errors/MissingPublishOptionError.scala index 85dfc6c17b..96143207c8 100644 --- a/modules/cli/src/main/scala/scala/cli/errors/MissingPublishOptionError.scala +++ b/modules/cli/src/main/scala/scala/cli/errors/MissingPublishOptionError.scala @@ -18,9 +18,12 @@ final class MissingPublishOptionError( if (configKeys.isEmpty) "" else s" or by setting ${configKeys.mkString(", ")} in the Scala CLI configuration" + val extraPart = + if (extraMessage.isEmpty) "" else s", ${extraMessage.dropWhile(_.isWhitespace)}" + s"Missing $name for publishing, specify one with $optionName" + directivePart + configPart + - extraMessage + extraPart } ) diff --git a/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala index 71879301a1..75d6e8e212 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala @@ -46,27 +46,29 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) os.rel / "org" / "virtuslab" / "scalacli" / "test" / s"simple_sjs1$scalaSuffix" / "0.2.0-SNAPSHOT" } - test("simple") { - val baseExpectedArtifacts = Seq( - s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT.pom", - s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT.jar", - s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT-javadoc.jar", - s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT-sources.jar" - ) - val expectedArtifacts = baseExpectedArtifacts - .flatMap { n => - Seq(n, n + ".asc") - } - .flatMap { n => - Seq("", ".md5", ".sha1").map(n + _) - } - .map(os.rel / _) - .toSet + val baseExpectedArtifacts = Seq( + s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT.pom", + s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT.jar", + s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT-javadoc.jar", + s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT-sources.jar" + ) - val expectedSourceEntries = Set( - "foo/Hello.scala", - "foo/Messages.scala" - ) + val expectedArtifacts = baseExpectedArtifacts + .flatMap { n => + Seq(n, n + ".asc") + } + .flatMap { n => + Seq("", ".md5", ".sha1").map(n + _) + } + .map(os.rel / _) + .toSet + + val expectedSourceEntries = Set( + "foo/Hello.scala", + "foo/Messages.scala" + ) + + test("simple") { val publicKey = { val uri = Thread.currentThread().getContextClassLoader @@ -271,4 +273,212 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) expect(mainClasses == Set(scalaFile1, scalaFile2, s"$scriptsDir.${scriptName}_sc")) } } + + test("missing secret key password") { + // format: off + val signingOptions = Seq( + "--secret-key", s"file:key.skr", + "--signer", "bc" + ) + // format: on + + TestCase.testInputs.fromRoot { root => + os.proc( + TestUtil.cli, + "--power", + "config", + "--remove", + "pgp.secret-key" + ).call(cwd = root) + + os.proc( + TestUtil.cli, + "--power", + "config", + "--remove", + "pgp.secret-key-password" + ).call(cwd = root) + + os.proc( + TestUtil.cli, + "--power", + "config", + "--remove", + "pgp.public-key" + ).call(cwd = root) + + os.proc( + TestUtil.cli, + "--power", + "pgp", + "create", + "--email", + "some_email", + "--password", + "value:" + ).call(cwd = root) + + val publicKey = os.Path("key.pub", root) + + os.proc( + TestUtil.cli, + "--power", + "publish", + extraOptions, + signingOptions, + "project", + "-R", + "test-repo" + ).call(cwd = root) + + val files = os.walk(root / "test-repo") + .filter(os.isFile(_)) + .map(_.relativeTo(root / "test-repo")) + val notInDir = files.filter(!_.startsWith(TestCase.expectedArtifactsDir)) + expect(notInDir.isEmpty) + + val files0 = files.map(_.relativeTo(TestCase.expectedArtifactsDir)).toSet + + expect((files0 -- expectedArtifacts).isEmpty) + expect((expectedArtifacts -- files0).isEmpty) + expect(files0 == expectedArtifacts) // just in case… + + val repoArgs = + Seq[os.Shellable]("-r", "!central", "-r", (root / "test-repo").toNIO.toUri.toASCIIString) + val dep = s"org.virtuslab.scalacli.test:simple${TestCase.scalaSuffix}:0.2.0-SNAPSHOT" + val res = os.proc(TestUtil.cs, "launch", repoArgs, dep).call(cwd = root) + val output = res.out.trim() + expect(output == "Hello") + + val sourceJarViaCsStr = + os.proc(TestUtil.cs, "fetch", repoArgs, "--sources", "--intransitive", dep) + .call(cwd = root) + .out.trim() + val sourceJarViaCs = os.Path(sourceJarViaCsStr, os.pwd) + val zf = new ZipFile(sourceJarViaCs.toIO) + val entries = zf.entries().asScala.toVector.map(_.getName).toSet + expect(entries == expectedSourceEntries) + + val signatures = expectedArtifacts.filter(_.last.endsWith(".asc")) + assert(signatures.nonEmpty) + os.proc( + TestUtil.cli, + "--power", + "pgp", + "verify", + "--key", + publicKey, + signatures.map(os.rel / "test-repo" / TestCase.expectedArtifactsDir / _) + ).call(cwd = root) + } + } + + test("secret keys in config") { + + TestCase.testInputs.fromRoot { root => + os.proc( + TestUtil.cli, + "--power", + "config", + "--remove", + "pgp.secret-key" + ).call(cwd = root) + + os.proc( + TestUtil.cli, + "--power", + "config", + "--remove", + "pgp.secret-key-password" + ).call(cwd = root) + + os.proc( + TestUtil.cli, + "--power", + "config", + "--remove", + "pgp.public-key" + ).call(cwd = root) + + os.proc( + TestUtil.cli, + "--power", + "config", + "--create-pgp-key" + ).call(cwd = root) + + TestCase.testInputs.fromRoot { root => + os.proc( + TestUtil.cli, + "--power", + "publish", + extraOptions, + "--signer", + "bc", + "project", + "-R", + "test-repo" + ).call( + cwd = root, + stdin = os.Inherit, + stdout = os.Inherit + ) + + val files = os.walk(root / "test-repo") + .filter(os.isFile(_)) + .map(_.relativeTo(root / "test-repo")) + val notInDir = files.filter(!_.startsWith(TestCase.expectedArtifactsDir)) + expect(notInDir.isEmpty) + + val files0 = files.map(_.relativeTo(TestCase.expectedArtifactsDir)).toSet + + expect((files0 -- expectedArtifacts).isEmpty) + expect((expectedArtifacts -- files0).isEmpty) + expect(files0 == expectedArtifacts) // just in case… + + val repoArgs = + Seq[os.Shellable]("-r", "!central", "-r", (root / "test-repo").toNIO.toUri.toASCIIString) + val dep = s"org.virtuslab.scalacli.test:simple${TestCase.scalaSuffix}:0.2.0-SNAPSHOT" + val res = os.proc(TestUtil.cs, "launch", repoArgs, dep).call(cwd = root) + val output = res.out.trim() + expect(output == "Hello") + + val sourceJarViaCsStr = + os.proc(TestUtil.cs, "fetch", repoArgs, "--sources", "--intransitive", dep) + .call(cwd = root) + .out.trim() + val sourceJarViaCs = os.Path(sourceJarViaCsStr, os.pwd) + val zf = new ZipFile(sourceJarViaCs.toIO) + val entries = zf.entries().asScala.toVector.map(_.getName).toSet + expect(entries == expectedSourceEntries) + + val publicKey = os.proc( + TestUtil.cli, + "--power", + "config", + "pgp.public-key" + ).call(cwd = root) + .out.trim() + .stripPrefix("value:") + + os.write(os.Path("key.pub", root), publicKey) + + println(os.list(root)) + + val signatures = expectedArtifacts.filter(_.last.endsWith(".asc")) + assert(signatures.nonEmpty) + os.proc( + TestUtil.cli, + "--power", + "pgp", + "verify", + "--key", + s"key.pub", + signatures.map(os.rel / "test-repo" / TestCase.expectedArtifactsDir / _) + ) + .call(cwd = root) + } + } + } + } diff --git a/modules/integration/src/test/scala/scala/cli/integration/PublishTestsDefault.scala b/modules/integration/src/test/scala/scala/cli/integration/PublishTestsDefault.scala index 3f7c20e719..e007e2ea44 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/PublishTestsDefault.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/PublishTestsDefault.scala @@ -299,12 +299,13 @@ class PublishTestsDefault extends PublishTestDefinitions(scalaVersionOpt = None) ) ) inputs.fromRoot { root => - val failRes = os.proc(TestUtil.cli, "--power", "publish", "--dummy", "messages") - .call(cwd = root, mergeErrIntoOut = true) + val failRes = + os.proc(TestUtil.cli, "--power", "publish", "--dummy", "--signer", "none", "messages") + .call(cwd = root, mergeErrIntoOut = true) checkWarnings(failRes.out.text(), hasWarnings = true) checkCredentialsWarning(failRes.out.text()) - val okRes = os.proc(TestUtil.cli, "--power", "publish", "--dummy", ".") + val okRes = os.proc(TestUtil.cli, "--power", "publish", "--dummy", "--signer", "none", ".") .call(cwd = root, mergeErrIntoOut = true) checkWarnings(okRes.out.text(), hasWarnings = false) checkCredentialsWarning(okRes.out.text()) From beb4f279ac7959b7e7dd0f4e507c9cacb70b5cff Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Tue, 7 Mar 2023 09:11:03 +0100 Subject: [PATCH 2/9] NIT change message texts, refactor url making --- .../src/main/scala/scala/cli/commands/publish/Publish.scala | 4 ++-- .../scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala index 2f76d9de5e..f28c5a0e32 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala @@ -948,7 +948,7 @@ object Publish extends ScalaCommand[PublishOptions] with BuildCommandHelpers { ) } yield getBouncyCastleSigner(secretKey, getSecretKeyPasswordOpt) case _ => - logger.diagnostic("Not signing files as it's not needed nor has it been specified") + logger.message("Artifacts not signed as it's not required nor has it been specified") Right(NopSigner) } @@ -1074,7 +1074,7 @@ object Publish extends ScalaCommand[PublishOptions] with BuildCommandHelpers { Seq(mod.organization.value, mod.name.value, version) else mod.organization.value.split('.').toSeq ++ Seq(mod.name.value, version) - elems.map("/" + _).mkString + "/" + elems.mkString("/", "/", "/") } val path = { val url = checkRepo.root.stripSuffix("/") + relPath diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala index d543042457..dd0d6c5a07 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala @@ -91,7 +91,7 @@ final case class PgpSecretKeyCheck( options.randomSecretKeyMail .toRight( new MissingPublishOptionError( - "random secret key mail", + "the e-mail address to associate to the random key pair", "--random-secret-key-mail", "" ) From a1dfbb570afce32be6fc7c3420a4c1350b80531f Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Tue, 7 Mar 2023 10:11:39 +0100 Subject: [PATCH 3/9] NIT remove hardcoded config key --- .../cli/src/main/scala/scala/cli/commands/publish/Publish.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala index f28c5a0e32..93955667a0 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala @@ -942,7 +942,7 @@ object Publish extends ScalaCommand[PublishOptions] with BuildCommandHelpers { "secret key", "--secret-key", directiveName = "", - configKeys = Seq("pgp.secret-key"), + configKeys = Seq(Keys.pgpSecretKey.fullName), extraMessage = shouldSignMsg ) ) From 6d1635cdb92c0ff04a5272c34f961ce45dd2dd79 Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Tue, 7 Mar 2023 10:47:54 +0100 Subject: [PATCH 4/9] Add checking for secret key in config in publish setup --- .../cli/commands/publish/checks/PgpSecretKeyCheck.scala | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala index dd0d6c5a07..50b740987a 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala @@ -35,8 +35,15 @@ final case class PgpSecretKeyCheck( def check(pubOpt: BPublishOptions): Boolean = { val opt0 = pubOpt.retained(options.publishParams.setupCi) + + lazy val configSecretKey = for { + secretKeyOpt <- configDb().get(Keys.pgpSecretKey).wrapConfigException.toOption + secretKey <- secretKeyOpt + } yield secretKey + opt0.repository.orElse(options.publishRepo.publishRepository).contains("github") || - opt0.secretKey.isDefined + opt0.secretKey.isDefined || + (options.publishParams.ci.contains(false) && configSecretKey.isDefined) } private val base64Chars = (('A' to 'Z') ++ ('a' to 'z') ++ ('0' to '9') ++ Seq('+', '/', '=')) From a5082203a7e5b438b5bbbea6d487fa7979775787 Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Tue, 7 Mar 2023 10:51:05 +0100 Subject: [PATCH 5/9] NIT remove Scala CLI hardcoded reference --- .../scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala | 2 +- .../main/scala/scala/cli/errors/MissingPublishOptionError.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala index 50b740987a..b8579b7dfa 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/publish/checks/PgpSecretKeyCheck.scala @@ -143,7 +143,7 @@ final case class PgpSecretKeyCheck( "publish.secretKey", configKeys = Seq(Keys.pgpSecretKey.fullName), extraMessage = - ", and specify publish.secretKeyPassword / --secret-key-password if needed." + + "also specify publish.secretKeyPassword / --secret-key-password if needed." + (if (options.publishParams.setupCi) " Alternatively, pass --random-secret-key" else "") diff --git a/modules/cli/src/main/scala/scala/cli/errors/MissingPublishOptionError.scala b/modules/cli/src/main/scala/scala/cli/errors/MissingPublishOptionError.scala index 96143207c8..8a0a052b57 100644 --- a/modules/cli/src/main/scala/scala/cli/errors/MissingPublishOptionError.scala +++ b/modules/cli/src/main/scala/scala/cli/errors/MissingPublishOptionError.scala @@ -17,7 +17,7 @@ final class MissingPublishOptionError( val configPart = if (configKeys.isEmpty) "" else - s" or by setting ${configKeys.mkString(", ")} in the Scala CLI configuration" + s" or by setting ${configKeys.mkString(", ")} in the configuration" val extraPart = if (extraMessage.isEmpty) "" else s", ${extraMessage.dropWhile(_.isWhitespace)}" From fddf3501a3206def580511134468abfbf220ddb1 Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Tue, 7 Mar 2023 11:17:01 +0100 Subject: [PATCH 6/9] NIT make expected artifacts a set --- .../scala/scala/cli/integration/PublishTestDefinitions.scala | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala index 75d6e8e212..8e07b4d76f 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala @@ -46,7 +46,7 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) os.rel / "org" / "virtuslab" / "scalacli" / "test" / s"simple_sjs1$scalaSuffix" / "0.2.0-SNAPSHOT" } - val baseExpectedArtifacts = Seq( + val baseExpectedArtifacts = Set( s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT.pom", s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT.jar", s"simple${TestCase.scalaSuffix}-0.2.0-SNAPSHOT-javadoc.jar", @@ -61,7 +61,6 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) Seq("", ".md5", ".sha1").map(n + _) } .map(os.rel / _) - .toSet val expectedSourceEntries = Set( "foo/Hello.scala", From c7d4a8a4a2d4d5165bf01fe27064413d1a932fd9 Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Tue, 7 Mar 2023 15:00:59 +0100 Subject: [PATCH 7/9] Add test-only scala-cli config --- .../integration/PublishTestDefinitions.scala | 78 +++++++------------ 1 file changed, 26 insertions(+), 52 deletions(-) diff --git a/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala index 8e07b4d76f..461ad68e11 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala @@ -6,6 +6,7 @@ import java.nio.file.Paths import java.util.zip.ZipFile import scala.jdk.CollectionConverters.* +import scala.util.Properties abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) extends ScalaCliSuite with TestScalaVersionArgs { @@ -282,29 +283,15 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) // format: on TestCase.testInputs.fromRoot { root => - os.proc( - TestUtil.cli, - "--power", - "config", - "--remove", - "pgp.secret-key" - ).call(cwd = root) + val confDir = root / "config" + val confFile = confDir / "test-config.json" - os.proc( - TestUtil.cli, - "--power", - "config", - "--remove", - "pgp.secret-key-password" - ).call(cwd = root) + os.write(confFile, "{}", createFolders = true) - os.proc( - TestUtil.cli, - "--power", - "config", - "--remove", - "pgp.public-key" - ).call(cwd = root) + if (!Properties.isWin) + os.perms.set(confDir, "rwx------") + + val extraEnv = Map("SCALA_CLI_CONFIG" -> confFile.toString) os.proc( TestUtil.cli, @@ -315,7 +302,7 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) "some_email", "--password", "value:" - ).call(cwd = root) + ).call(cwd = root, env = extraEnv) val publicKey = os.Path("key.pub", root) @@ -328,7 +315,7 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) "project", "-R", "test-repo" - ).call(cwd = root) + ).call(cwd = root, env = extraEnv) val files = os.walk(root / "test-repo") .filter(os.isFile(_)) @@ -368,43 +355,31 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) "--key", publicKey, signatures.map(os.rel / "test-repo" / TestCase.expectedArtifactsDir / _) - ).call(cwd = root) + ).call(cwd = root, env = extraEnv) } } test("secret keys in config") { TestCase.testInputs.fromRoot { root => - os.proc( - TestUtil.cli, - "--power", - "config", - "--remove", - "pgp.secret-key" - ).call(cwd = root) + val confDir = root / "config" + val confFile = confDir / "test-config.json" - os.proc( - TestUtil.cli, - "--power", - "config", - "--remove", - "pgp.secret-key-password" - ).call(cwd = root) + os.write(confFile, "{}", createFolders = true) - os.proc( - TestUtil.cli, - "--power", - "config", - "--remove", - "pgp.public-key" - ).call(cwd = root) + if (!Properties.isWin) + os.perms.set(confDir, "rwx------") + + val extraEnv = Map("SCALA_CLI_CONFIG" -> confFile.toString) os.proc( TestUtil.cli, "--power", "config", - "--create-pgp-key" - ).call(cwd = root) + "--create-pgp-key", + "--email", + "some_email" + ).call(cwd = root, env = extraEnv) TestCase.testInputs.fromRoot { root => os.proc( @@ -420,7 +395,8 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) ).call( cwd = root, stdin = os.Inherit, - stdout = os.Inherit + stdout = os.Inherit, + env = extraEnv ) val files = os.walk(root / "test-repo") @@ -456,14 +432,12 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) "--power", "config", "pgp.public-key" - ).call(cwd = root) + ).call(cwd = root, env = extraEnv) .out.trim() .stripPrefix("value:") os.write(os.Path("key.pub", root), publicKey) - println(os.list(root)) - val signatures = expectedArtifacts.filter(_.last.endsWith(".asc")) assert(signatures.nonEmpty) os.proc( @@ -475,7 +449,7 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) s"key.pub", signatures.map(os.rel / "test-repo" / TestCase.expectedArtifactsDir / _) ) - .call(cwd = root) + .call(cwd = root, env = extraEnv) } } } From c1b85d4af4262d658547fcac23fa7da073027b86 Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Tue, 7 Mar 2023 15:32:14 +0100 Subject: [PATCH 8/9] NIT Make output cooler and not visible when signer=none passed --- .../src/main/scala/scala/cli/commands/publish/Publish.scala | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala index 93955667a0..ab2f23efdd 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala @@ -948,7 +948,10 @@ object Publish extends ScalaCommand[PublishOptions] with BuildCommandHelpers { ) } yield getBouncyCastleSigner(secretKey, getSecretKeyPasswordOpt) case _ => - logger.message("Artifacts not signed as it's not required nor has it been specified") + if (!publishOptions.contextual(isCi).signer.contains(PSigner.Nop)) + logger.message( + "\ud83d\udd13 Artifacts NOT signed as it's not required nor has it been specified" + ) Right(NopSigner) } From 6050d292e4bd18976d20a20778a79720854a4143 Mon Sep 17 00:00:00 2001 From: Maciej Gajek Date: Wed, 8 Mar 2023 09:58:24 +0100 Subject: [PATCH 9/9] Exclude no password tests for native, add warning --- .../scala/cli/commands/publish/Publish.scala | 3 + .../integration/PublishTestDefinitions.scala | 141 +++++++++--------- 2 files changed, 74 insertions(+), 70 deletions(-) diff --git a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala index ab2f23efdd..614809b5a7 100644 --- a/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala +++ b/modules/cli/src/main/scala/scala/cli/commands/publish/Publish.scala @@ -865,6 +865,9 @@ object Publish extends ScalaCommand[PublishOptions] with BuildCommandHelpers { } } + if (secretKeyPasswordOpt.isEmpty) + logger.diagnostic("PGP signing with no password is not recommended since it's not stable") + if (forceSigningBinary) (new scala.cli.internal.BouncycastleSignerMakerSubst).get( secretKeyPasswordOpt.fold(null)(_.toCliSigning), diff --git a/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala index 461ad68e11..2a993c310c 100644 --- a/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala +++ b/modules/integration/src/test/scala/scala/cli/integration/PublishTestDefinitions.scala @@ -274,90 +274,91 @@ abstract class PublishTestDefinitions(val scalaVersionOpt: Option[String]) } } - test("missing secret key password") { - // format: off - val signingOptions = Seq( - "--secret-key", s"file:key.skr", - "--signer", "bc" - ) - // format: on + if (!TestUtil.isNativeCli) + test("missing secret key password") { + // format: off + val signingOptions = Seq( + "--secret-key", s"file:key.skr", + "--signer", "bc" + ) + // format: on - TestCase.testInputs.fromRoot { root => - val confDir = root / "config" - val confFile = confDir / "test-config.json" + TestCase.testInputs.fromRoot { root => + val confDir = root / "config" + val confFile = confDir / "test-config.json" - os.write(confFile, "{}", createFolders = true) + os.write(confFile, "{}", createFolders = true) - if (!Properties.isWin) - os.perms.set(confDir, "rwx------") + if (!Properties.isWin) + os.perms.set(confDir, "rwx------") - val extraEnv = Map("SCALA_CLI_CONFIG" -> confFile.toString) + val extraEnv = Map("SCALA_CLI_CONFIG" -> confFile.toString) - os.proc( - TestUtil.cli, - "--power", - "pgp", - "create", - "--email", - "some_email", - "--password", - "value:" - ).call(cwd = root, env = extraEnv) + os.proc( + TestUtil.cli, + "--power", + "pgp", + "create", + "--email", + "some_email", + "--password", + "value:" + ).call(cwd = root, env = extraEnv) - val publicKey = os.Path("key.pub", root) + val publicKey = os.Path("key.pub", root) - os.proc( - TestUtil.cli, - "--power", - "publish", - extraOptions, - signingOptions, - "project", - "-R", - "test-repo" - ).call(cwd = root, env = extraEnv) + os.proc( + TestUtil.cli, + "--power", + "publish", + extraOptions, + signingOptions, + "project", + "-R", + "test-repo" + ).call(cwd = root, env = extraEnv) - val files = os.walk(root / "test-repo") - .filter(os.isFile(_)) - .map(_.relativeTo(root / "test-repo")) - val notInDir = files.filter(!_.startsWith(TestCase.expectedArtifactsDir)) - expect(notInDir.isEmpty) + val files = os.walk(root / "test-repo") + .filter(os.isFile(_)) + .map(_.relativeTo(root / "test-repo")) + val notInDir = files.filter(!_.startsWith(TestCase.expectedArtifactsDir)) + expect(notInDir.isEmpty) - val files0 = files.map(_.relativeTo(TestCase.expectedArtifactsDir)).toSet + val files0 = files.map(_.relativeTo(TestCase.expectedArtifactsDir)).toSet - expect((files0 -- expectedArtifacts).isEmpty) - expect((expectedArtifacts -- files0).isEmpty) - expect(files0 == expectedArtifacts) // just in case… + expect((files0 -- expectedArtifacts).isEmpty) + expect((expectedArtifacts -- files0).isEmpty) + expect(files0 == expectedArtifacts) // just in case… - val repoArgs = - Seq[os.Shellable]("-r", "!central", "-r", (root / "test-repo").toNIO.toUri.toASCIIString) - val dep = s"org.virtuslab.scalacli.test:simple${TestCase.scalaSuffix}:0.2.0-SNAPSHOT" - val res = os.proc(TestUtil.cs, "launch", repoArgs, dep).call(cwd = root) - val output = res.out.trim() - expect(output == "Hello") + val repoArgs = + Seq[os.Shellable]("-r", "!central", "-r", (root / "test-repo").toNIO.toUri.toASCIIString) + val dep = s"org.virtuslab.scalacli.test:simple${TestCase.scalaSuffix}:0.2.0-SNAPSHOT" + val res = os.proc(TestUtil.cs, "launch", repoArgs, dep).call(cwd = root) + val output = res.out.trim() + expect(output == "Hello") - val sourceJarViaCsStr = - os.proc(TestUtil.cs, "fetch", repoArgs, "--sources", "--intransitive", dep) - .call(cwd = root) - .out.trim() - val sourceJarViaCs = os.Path(sourceJarViaCsStr, os.pwd) - val zf = new ZipFile(sourceJarViaCs.toIO) - val entries = zf.entries().asScala.toVector.map(_.getName).toSet - expect(entries == expectedSourceEntries) + val sourceJarViaCsStr = + os.proc(TestUtil.cs, "fetch", repoArgs, "--sources", "--intransitive", dep) + .call(cwd = root) + .out.trim() + val sourceJarViaCs = os.Path(sourceJarViaCsStr, os.pwd) + val zf = new ZipFile(sourceJarViaCs.toIO) + val entries = zf.entries().asScala.toVector.map(_.getName).toSet + expect(entries == expectedSourceEntries) - val signatures = expectedArtifacts.filter(_.last.endsWith(".asc")) - assert(signatures.nonEmpty) - os.proc( - TestUtil.cli, - "--power", - "pgp", - "verify", - "--key", - publicKey, - signatures.map(os.rel / "test-repo" / TestCase.expectedArtifactsDir / _) - ).call(cwd = root, env = extraEnv) + val signatures = expectedArtifacts.filter(_.last.endsWith(".asc")) + assert(signatures.nonEmpty) + os.proc( + TestUtil.cli, + "--power", + "pgp", + "verify", + "--key", + publicKey, + signatures.map(os.rel / "test-repo" / TestCase.expectedArtifactsDir / _) + ).call(cwd = root, env = extraEnv) + } } - } test("secret keys in config") {