ome / openmicroscopy

OME (Open Microscopy Environment) develops open-source software and data format standards for the storage and manipulation of biological light microscopy data. A joint project between universities, research establishments and industry in Europe and the USA, OME has over 20 active researchers with strong links to the microscopy community. Funded by private and public research grants, OME has been a major force on the international microscopy stage since 2000.
https://www.openmicroscopy.org/omero
GNU General Public License v2.0
200 stars 102 forks source link

Distribution contents regression with Ivy 2.5.2 #6405

Closed chris-allan closed 3 months ago

chris-allan commented 3 months ago

After the release of 5.6.12 it was reported ^1 that old artifacts, some with critical security vulnerabilities such log4j 1.2.17, were ending up in the OMERO.server distribution. This appears to an unintended consequence of #6388. The diff of JARs from v5.6.12 with 94c9b166741ca01ebf2bffcdb21c43f10bb93b6a reverted is as follows:

--- 5_6_12_server_jars  2024-08-06 09:43:14.683495565 +0000
+++ latest_jars 2024-08-06 09:42:52.459392636 +0000
@@ -40,8 +40,6 @@
 batik-xml.jar
 btm.jar
 bufr.jar
-byte-buddy-agent.jar
-byte-buddy.jar
 c3p0.jar
 calcite-core.jar
 calcite-linq4j.jar
@@ -102,7 +100,6 @@
 jblosc.jar
 jcip-annotations.jar
 jcl-over-slf4j.jar
-jcommander.jar
 jgoodies-common.jar
 jgoodies-forms.jar
 jhdf5.jar
@@ -118,7 +115,6 @@
 jzarr.jar
 kryo.jar
 listenablefuture.jar
-log4j.jar
 logback-classic.jar
 logback-core.jar
 lucene-analyzers.jar
@@ -139,8 +135,6 @@
 metrics-logback.jar
 minio.jar
 minlog.jar
-mockito-core.jar
-mockito-inline.jar
 native-lib-loader.jar
 objenesis.jar
 okhttp.jar
@@ -169,8 +163,6 @@
 serializer.jar
 sketches-core.jar
 slf4j-api.jar
-slf4j-jdk14.jar
-slf4j-log4j12.jar
 snakeyaml.jar
 solr-analysis-extras.jar
 solr-commons-csv.jar
@@ -193,7 +185,6 @@
 spring-tx.jar
 sqlite-jdbc.jar
 subethasmtp.jar
-testng.jar
 turbojpeg.jar
 udunits.jar
 xalan.jar

My running theory is this is due to changes in dependency resolution that result in artifacts from testing targets leaking into the runtime ones.

imagesc-bot commented 3 months ago

This issue has been mentioned on Image.sc Forum. There might be relevant details there:

https://forum.image.sc/t/critical-old-and-unsafe-log4j-jar-file-in-omero-5-6-12/100016/2

chris-allan commented 3 months ago

Diff of the client JARs:

--- 5_6_12_client_jars  2024-08-06 10:21:44.699172031 +0000
+++ latest_client_jars  2024-08-06 10:22:02.367277186 +0000
@@ -40,8 +40,6 @@
 batik-xml.jar
 btm.jar
 bufr.jar
-byte-buddy-agent.jar
-byte-buddy.jar
 c3p0.jar
 calcite-core.jar
 calcite-linq4j.jar
@@ -102,7 +100,6 @@
 jblosc.jar
 jcip-annotations.jar
 jcl-over-slf4j.jar
-jcommander.jar
 jgoodies-common.jar
 jgoodies-forms.jar
 jhdf5.jar
@@ -118,7 +115,6 @@
 jzarr.jar
 kryo.jar
 listenablefuture.jar
-log4j.jar
 logback-classic.jar
 logback-core.jar
 lucene-analyzers.jar
@@ -139,8 +135,6 @@
 metrics-logback.jar
 minio.jar
 minlog.jar
-mockito-core.jar
-mockito-inline.jar
 native-lib-loader.jar
 objenesis.jar
 okhttp.jar
@@ -169,8 +163,6 @@
 serializer.jar
 sketches-core.jar
 slf4j-api.jar
-slf4j-jdk14.jar
-slf4j-log4j12.jar
 snakeyaml.jar
 solr-analysis-extras.jar
 solr-commons-csv.jar
@@ -193,7 +185,6 @@
 spring-tx.jar
 sqlite-jdbc.jar
 subethasmtp.jar
-testng.jar
 turbojpeg.jar
 udunits.jar
 xalan.jar
chris-allan commented 3 months ago

latest_jars and latest_client_jars were produced with a build using #6406.

chris-allan commented 3 months ago

JARs that can be safely removed from a 5.6.12 distribution are:

lib/server/byte-buddy-agent.jar
lib/server/byte-buddy.jar
lib/server/jcommander.jar
lib/server/log4j.jar
lib/server/mockito-core.jar
lib/server/mockito-inline.jar
lib/server/slf4j-jdk14.jar
lib/server/slf4j-log4j12.jar
lib/server/testng.jar

lib/client/byte-buddy-agent.jar
lib/client/byte-buddy.jar
lib/client/jcommander.jar
lib/client/log4j.jar
lib/client/mockito-core.jar
lib/client/mockito-inline.jar
lib/client/slf4j-jdk14.jar
lib/client/slf4j-log4j12.jar
lib/client/testng.jar
sbesson commented 3 months ago

Thanks all for flagging this regression and the initial investigation made last week.

This initially came as a surprise as I am confident that I had checked the client & server JARs while working on this upgrade. Checking out v5.6.12 and re-running ./build.py clean build-dev earlier, I was also unable to reproduce this issue at first and the dist/lib/server and dist/lib/client folders contained neither log4j, jcommander or other test dependencies.

As highlighted by this thread, some significant changes have been made in Ivy in between versions 2.4.0 and 2.5.0. Even though the build was running with Ivy 2.5.2, one issue is that my local Maven repository cache under ~/.m2 was populated with dependencies fetched using Ivy 2.4.0. After cleaning my local ~/.m2 directory, a re-execution of ./build.py clean build-dev forced a fresh resolution of all artifacts from scratch and was sufficient to reproduce the issue above i.e. log4j.jar is now copied under dist/lib/client and dist/lib/server. This observation is consistent with the fact that the GitHub workflow generating the release artifacts is using a clean Maven workspace.

Trying to understand the source of this regression, an analysis of the dependencies shows that the log4j artifact is (incorrectly) fetched as a dependency of jul-to-slf4j declared in the top-level ivy.xml file. Comparing the two Maven repository caches (~/.m2.bak formerly populated using Ivy 2.4.0 and ~/.m2 populated using Ivy 2.5.2), the primary difference can be identified in the content of the Ivy XML files

sbesson@Sebastiens-MacBook-Pro-3 openmicroscopy % diff ~/.m2/repository/org/slf4j/jul-to-slf4j/1.7.30/ivy-1.7.30.xml ~/.m2.bak/repository/org/slf4j/jul-to-slf4j/1.7.30/ivy-1.7.30.xml 
9d8
<               <license name="MIT License" url="http://www.opensource.org/licenses/mit-license.php" />
40c39
<               <conf name="test" visibility="public" description="this scope indicates that the dependency is not required for normal use of the application, and is only available for the test compilation and execution phases." extends="runtime"/>
---
>               <conf name="test" visibility="private" description="this scope indicates that the dependency is not required for normal use of the application, and is only available for the test compilation and execution phases." extends="runtime"/>
55,56d53
<               <override org="org.slf4j" module="slf4j-api" matcher="exact" rev="1.7.30"/>
<               <override org="org.slf4j" module="slf4j-jdk14" matcher="exact" rev="1.7.30"/>
58a56,57
>               <override org="org.slf4j" module="slf4j-jdk14" matcher="exact" rev="1.7.30"/>
>               <override org="org.slf4j" module="slf4j-api" matcher="exact" rev="1.7.30"/>

This diff is consistent with the change of visibility for the test configuration described in the Ivy 2.5.0 release notes

FIX: Made the Maven 'test' configuration public so we can use the test-jar as dependency ([IVY-1444](https://issues.apache.org/jira/browse/IVY-1444))

As suggested in the issue description, this change in configuration visibility currently causes some dependencies declared at the test scope to be resolved for the client and server configurations. Next step will be to review the chain of configuration mapping when invoking these targets. Semi-related, jul-to-slf4j and jcl-to-slf4j should likely be bumped to version 2.0.x especially as the underlying slf4j-api dependency is now 2.x