From 606455d321e7cca6f8df1bba192370b42a90ffa7 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 11 Apr 2024 15:06:44 +0200 Subject: [PATCH 1/5] [MGPG-125] Fix "bestPractices" As currently due defaulted values it would always fail, unless user explicitly re-configures plugin. --- https://issues.apache.org/jira/browse/MGPG-125 --- .../invoker.properties | 18 +++ .../sign-release-best-practices-fail/pom.xml | 103 ++++++++++++++++++ .../verify.groovy | 29 +++++ .../invoker.properties | 18 +++ src/it/sign-release-best-practices/pom.xml | 100 +++++++++++++++++ .../sign-release-best-practices/verify.groovy | 38 +++++++ .../maven/plugins/gpg/AbstractGpgMojo.java | 20 +++- 7 files changed, 320 insertions(+), 6 deletions(-) create mode 100644 src/it/sign-release-best-practices-fail/invoker.properties create mode 100644 src/it/sign-release-best-practices-fail/pom.xml create mode 100644 src/it/sign-release-best-practices-fail/verify.groovy create mode 100644 src/it/sign-release-best-practices/invoker.properties create mode 100644 src/it/sign-release-best-practices/pom.xml create mode 100644 src/it/sign-release-best-practices/verify.groovy diff --git a/src/it/sign-release-best-practices-fail/invoker.properties b/src/it/sign-release-best-practices-fail/invoker.properties new file mode 100644 index 0000000..f2a7dfb --- /dev/null +++ b/src/it/sign-release-best-practices-fail/invoker.properties @@ -0,0 +1,18 @@ +# 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. + +invoker.buildResult = failure diff --git a/src/it/sign-release-best-practices-fail/pom.xml b/src/it/sign-release-best-practices-fail/pom.xml new file mode 100644 index 0000000..38223b4 --- /dev/null +++ b/src/it/sign-release-best-practices-fail/pom.xml @@ -0,0 +1,103 @@ + + + + + + 4.0.0 + + org.apache.maven.its.gpg.sr + test + 1.0 + jar + + + Tests the installation of a simple release JAR with an attached artifact and its signatures. + + + + true + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 2.0.2 + + + org.apache.maven.plugins + maven-gpg-plugin + @project.version@ + + + sign-artifacts + + sign + + + + + true + + + gpg.passphrase-sign-with-passphase-from-maven-settings + + + + org.apache.maven.plugins + maven-install-plugin + 2.2 + + true + + + + org.apache.maven.plugins + maven-jar-plugin + 2.1 + + + org.apache.maven.plugins + maven-resources-plugin + 2.2 + + + org.apache.maven.plugins + maven-source-plugin + 2.0.4 + + + attach-sources + + jar + + + + + + org.apache.maven.plugins + maven-surefire-plugin + 2.3.1 + + + + + diff --git a/src/it/sign-release-best-practices-fail/verify.groovy b/src/it/sign-release-best-practices-fail/verify.groovy new file mode 100644 index 0000000..d742a34 --- /dev/null +++ b/src/it/sign-release-best-practices-fail/verify.groovy @@ -0,0 +1,29 @@ +/* + * 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. + */ + + +import org.codehaus.plexus.util.FileUtils + +var buildLog = new File(basedir, "build.log") +var logContent = FileUtils.fileRead(buildLog) + +// assert that bestPractice+worstPractice => MojoFailure +if (!logContent.contains("MojoFailureException: Do not store passphrase in any file")) { + throw new Exception("Maven build did not fail in consequence of bestPractices+worstPractices") +} diff --git a/src/it/sign-release-best-practices/invoker.properties b/src/it/sign-release-best-practices/invoker.properties new file mode 100644 index 0000000..1122205 --- /dev/null +++ b/src/it/sign-release-best-practices/invoker.properties @@ -0,0 +1,18 @@ +# 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. + +invoker.environmentVariables.MAVEN_GPG_PASSPHRASE = TEST diff --git a/src/it/sign-release-best-practices/pom.xml b/src/it/sign-release-best-practices/pom.xml new file mode 100644 index 0000000..d19538d --- /dev/null +++ b/src/it/sign-release-best-practices/pom.xml @@ -0,0 +1,100 @@ + + + + + + 4.0.0 + + org.apache.maven.its.gpg.sr + test + 1.0 + jar + + + Tests the installation of a simple release JAR with an attached artifact and its signatures. + + + + true + + + + + + org.apache.maven.plugins + maven-compiler-plugin + 2.0.2 + + + org.apache.maven.plugins + maven-gpg-plugin + @project.version@ + + + sign-artifacts + + sign + + + + + true + + + + org.apache.maven.plugins + maven-install-plugin + 2.2 + + true + + + + org.apache.maven.plugins + maven-jar-plugin + 2.1 + + + org.apache.maven.plugins + maven-resources-plugin + 2.2 + + + org.apache.maven.plugins + maven-source-plugin + 2.0.4 + + + attach-sources + + jar + + + + + + org.apache.maven.plugins + maven-surefire-plugin + 2.3.1 + + + + + diff --git a/src/it/sign-release-best-practices/verify.groovy b/src/it/sign-release-best-practices/verify.groovy new file mode 100644 index 0000000..489b1b3 --- /dev/null +++ b/src/it/sign-release-best-practices/verify.groovy @@ -0,0 +1,38 @@ +/* + * 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. + */ +var artifactDir = new File(localRepositoryPath, "org/apache/maven/its/gpg/sr/test/1.0") + +var expectedFiles = [ + "test-1.0.pom", + "test-1.0.pom.asc", + "test-1.0.jar", + "test-1.0.jar.asc", + "test-1.0-sources.jar", + "test-1.0-sources.jar.asc" +] + +for (String expectedFile : expectedFiles) { + var file = new File(artifactDir, expectedFile) + + println "Checking for existence of $file" + + if (!file.isFile()) { + throw new Exception("Missing file $file") + } +} diff --git a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java index 99469f3..1eddc50 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java @@ -131,13 +131,14 @@ public abstract class AbstractGpgMojo extends AbstractMojo { * Server id to lookup the passphrase under Maven settings. Do not use this parameter, it leaks * sensitive data. Passphrase should be provided only via gpg-agent or via env variable. * If parameter {@link #bestPractices} set to {@code true}, plugin fails when this parameter is configured. + * Is programatically defaulted to {@link #GPG_PASSPHRASE}. * * @since 1.6 * @deprecated Do not use this configuration, it may leak sensitive information. Rely on gpg-agent or env * variables instead. **/ @Deprecated - @Parameter(property = "gpg.passphraseServerId", defaultValue = GPG_PASSPHRASE) + @Parameter(property = "gpg.passphraseServerId") private String passphraseServerId; /** @@ -299,11 +300,18 @@ public final void execute() throws MojoExecutionException, MojoFailureException // We're skipping the signing stuff return; } - if (bestPractices && (isNotBlank(passphrase) || isNotBlank(passphraseServerId))) { - // Stop propagating worst practices: passphrase MUST NOT be in any file on disk - throw new MojoFailureException( - "Do not store passphrase in any file (disk or SCM repository), rely on GnuPG agent or provide passphrase in " - + passphraseEnvName + " environment variable."); + if (bestPractices) { + if (isNotBlank(passphrase) || isNotBlank(passphraseServerId)) { + // Stop propagating worst practices: passphrase MUST NOT be in any file on disk + throw new MojoFailureException( + "Do not store passphrase in any file (disk or SCM repository), rely on GnuPG agent or provide passphrase in " + + passphraseEnvName + " environment variable."); + } + } else { + if (passphraseServerId == null) { + // default it programmatically: this is needed to handle different cases re bestPractices + passphraseServerId = GPG_PASSPHRASE; + } } doExecute(); From b0aa01b9e7dde7c6ca3dcb9e944b1ce66e33561d Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Thu, 11 Apr 2024 16:28:53 +0200 Subject: [PATCH 2/5] Use same method for check --- src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java index 1eddc50..b5538f2 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java @@ -308,7 +308,7 @@ public final void execute() throws MojoExecutionException, MojoFailureException + passphraseEnvName + " environment variable."); } } else { - if (passphraseServerId == null) { + if (!isNotBlank(passphraseServerId)) { // default it programmatically: this is needed to handle different cases re bestPractices passphraseServerId = GPG_PASSPHRASE; } From ae95dc1bf55d33da9bb57d5f23b97b55a90ac967 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 12 Apr 2024 17:28:46 +0200 Subject: [PATCH 3/5] PR comments --- src/it/sign-release-best-practices-fail/pom.xml | 15 ++++++--------- .../verify.groovy | 11 +++-------- src/it/sign-release-best-practices/pom.xml | 15 ++++++--------- src/it/sign-release/verify.groovy | 4 +--- .../maven/plugins/gpg/AbstractGpgMojo.java | 17 +++++++++++------ 5 files changed, 27 insertions(+), 35 deletions(-) diff --git a/src/it/sign-release-best-practices-fail/pom.xml b/src/it/sign-release-best-practices-fail/pom.xml index 38223b4..812b973 100644 --- a/src/it/sign-release-best-practices-fail/pom.xml +++ b/src/it/sign-release-best-practices-fail/pom.xml @@ -40,7 +40,7 @@ under the License. org.apache.maven.plugins maven-compiler-plugin - 2.0.2 + @version.maven-compiler-plugin@ org.apache.maven.plugins @@ -64,25 +64,22 @@ under the License. org.apache.maven.plugins maven-install-plugin - 2.2 - - true - + @version.maven-install-plugin@ org.apache.maven.plugins maven-jar-plugin - 2.1 + @version.maven-jar-plugin@ org.apache.maven.plugins maven-resources-plugin - 2.2 + @version.maven-resources-plugin@ org.apache.maven.plugins maven-source-plugin - 2.0.4 + @version.maven-source-plugin@ attach-sources @@ -95,7 +92,7 @@ under the License. org.apache.maven.plugins maven-surefire-plugin - 2.3.1 + @version.maven-surefire@ diff --git a/src/it/sign-release-best-practices-fail/verify.groovy b/src/it/sign-release-best-practices-fail/verify.groovy index d742a34..0ad438c 100644 --- a/src/it/sign-release-best-practices-fail/verify.groovy +++ b/src/it/sign-release-best-practices-fail/verify.groovy @@ -17,13 +17,8 @@ * under the License. */ - -import org.codehaus.plexus.util.FileUtils - -var buildLog = new File(basedir, "build.log") -var logContent = FileUtils.fileRead(buildLog) +File buildLog = new File(basedir, "build.log") +String logContent = new File(basedir, "build.log").text // assert that bestPractice+worstPractice => MojoFailure -if (!logContent.contains("MojoFailureException: Do not store passphrase in any file")) { - throw new Exception("Maven build did not fail in consequence of bestPractices+worstPractices") -} +assert logContent.contains("MojoFailureException: Do not store passphrase in any file") diff --git a/src/it/sign-release-best-practices/pom.xml b/src/it/sign-release-best-practices/pom.xml index d19538d..cc6753f 100644 --- a/src/it/sign-release-best-practices/pom.xml +++ b/src/it/sign-release-best-practices/pom.xml @@ -40,7 +40,7 @@ under the License. org.apache.maven.plugins maven-compiler-plugin - 2.0.2 + @version.maven-compiler-plugin@ org.apache.maven.plugins @@ -61,25 +61,22 @@ under the License. org.apache.maven.plugins maven-install-plugin - 2.2 - - true - + @version.maven-install-plugin@ org.apache.maven.plugins maven-jar-plugin - 2.1 + @version.maven-jar-plugin@ org.apache.maven.plugins maven-resources-plugin - 2.2 + @version.maven-resources-plugin@ org.apache.maven.plugins maven-source-plugin - 2.0.4 + @version.maven-source-plugin@ attach-sources @@ -92,7 +89,7 @@ under the License. org.apache.maven.plugins maven-surefire-plugin - 2.3.1 + @version.maven-surefire@ diff --git a/src/it/sign-release/verify.groovy b/src/it/sign-release/verify.groovy index 489b1b3..cc27897 100644 --- a/src/it/sign-release/verify.groovy +++ b/src/it/sign-release/verify.groovy @@ -32,7 +32,5 @@ for (String expectedFile : expectedFiles) { println "Checking for existence of $file" - if (!file.isFile()) { - throw new Exception("Missing file $file") - } + assert file.isFile() } diff --git a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java index b5538f2..68ab280 100644 --- a/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java +++ b/src/main/java/org/apache/maven/plugins/gpg/AbstractGpgMojo.java @@ -301,12 +301,7 @@ public final void execute() throws MojoExecutionException, MojoFailureException return; } if (bestPractices) { - if (isNotBlank(passphrase) || isNotBlank(passphraseServerId)) { - // Stop propagating worst practices: passphrase MUST NOT be in any file on disk - throw new MojoFailureException( - "Do not store passphrase in any file (disk or SCM repository), rely on GnuPG agent or provide passphrase in " - + passphraseEnvName + " environment variable."); - } + enforceBestPractices(); } else { if (!isNotBlank(passphraseServerId)) { // default it programmatically: this is needed to handle different cases re bestPractices @@ -317,6 +312,16 @@ public final void execute() throws MojoExecutionException, MojoFailureException doExecute(); } + protected void enforceBestPractices() throws MojoFailureException { + // if any of those are not blank: meaning user did explicitly configure these + if (isNotBlank(passphrase) || isNotBlank(passphraseServerId)) { + // Stop propagating worst practices: passphrase MUST NOT be in any file on disk + throw new MojoFailureException( + "Do not store passphrase in any file (disk or SCM repository), rely on GnuPG agent or provide passphrase in " + + passphraseEnvName + " environment variable."); + } + } + protected abstract void doExecute() throws MojoExecutionException, MojoFailureException; private void logBestPracticeWarning(String source) { From 297f137e9a96780e396f89f824a3babdb74d9e53 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 12 Apr 2024 17:30:19 +0200 Subject: [PATCH 4/5] PR comments --- src/it/sign-release-best-practices/verify.groovy | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/it/sign-release-best-practices/verify.groovy b/src/it/sign-release-best-practices/verify.groovy index 489b1b3..cc27897 100644 --- a/src/it/sign-release-best-practices/verify.groovy +++ b/src/it/sign-release-best-practices/verify.groovy @@ -32,7 +32,5 @@ for (String expectedFile : expectedFiles) { println "Checking for existence of $file" - if (!file.isFile()) { - throw new Exception("Missing file $file") - } + assert file.isFile() } From c592cd998368bdf7eeb92beb7b026512b4f78df9 Mon Sep 17 00:00:00 2001 From: Tamas Cservenak Date: Fri, 12 Apr 2024 17:37:44 +0200 Subject: [PATCH 5/5] Strange, locally this is not needed? --- src/it/sign-release-best-practices/pom.xml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/it/sign-release-best-practices/pom.xml b/src/it/sign-release-best-practices/pom.xml index cc6753f..6bd9090 100644 --- a/src/it/sign-release-best-practices/pom.xml +++ b/src/it/sign-release-best-practices/pom.xml @@ -77,6 +77,9 @@ under the License. org.apache.maven.plugins maven-source-plugin @version.maven-source-plugin@ + + true + attach-sources