Ignore maven integration test poms (src/it/*/pom.xml)

[codefromthecrypt]

Description

Please ignore src/it/*/pom.xml files when analyzing for vulnerabilities.

Maven has a convention for integration tests, src/it/*/pom.xml, which are run with the maven-invoker-plugin. In zipkin, we use this style to ensure old clients can still pass tests. However, the old libraries configured in the integration tests are setting off vulnerability notices. I think they should not be treated as any priority as they will not result in production code.

Here's an example https://github.com/openzipkin/zipkin-reporter-java

If there is another way to hint to Trivy run by arbitrary users to not detect this (via something in our repo), an alternate is welcome!

Target

Git Repository

Scanner

Vulnerability

[nikpivkin]

Hi @codefromthecrypt !

You can use the --skip-files "**/src/it/*/pom.xml" flag to skip files https://aquasecurity.github.io/trivy/v0.48/docs/configuration/skipping/

[DmitriyLewen]

Hello @codefromthecrypt
@nikpivkin is right. We don't parse comments in pom.xml files.

About skipping these files by default:
Can you share docs with information, that integration tests should be saved in src/it folder.
I checked maven-invoker-plugin and only found information that maven-invoker-plugin project uses src/it for its integration tests, but no information that this is required.

[codefromthecrypt]

sure. I don't think that there is any way to change the relative directory path src/it when using the maven-invoker-plugin. Any project within that can have a pom file, but they are not deployable artifacts. Here's an example from their site: https://maven.apache.org/plugins/maven-invoker-plugin/usage.html

Another way to know this probably should be skipped is seeing that the version variables are templated. e.g. @old-okhttp.version@ as opposed to something maven would actually use normally like ${old-okhttp.version} https://github.com/openzipkin/zipkin-reporter-java/blob/master/okhttp3/src/it/okhttp3_v3/pom.xml#L63

In general, I think there are ways to heuristically check that this is an ignorable subproject by default. One way is to simply exclude src/it/*. To be more precise, you could look in the files you would ignore and know for sure you should ignore them based on maven-invoker-plugin template expressions like @ in the files.

[codefromthecrypt]

I opened this on one of our projects, but the more important one will be brave (zipkin instrumentation library), as it will have many examples of this, as it is similarly required to do interop testing with old versions, using maven-invoker-plugin to keep those versions out of production dependency trees. Opened for follow-up as I will be out for a while in a few days. openzipkin/zipkin-reporter-java#228

[DmitriyLewen]

Thanks for this information!

It really makes sense.
But I'm not sure if these changes won't affect other users (currently users can skip these files, but if we add skipping these files - they won't be able to stop skipping these files).

Let's wait a little and see what users think.
If this skip is required, we will add it.

[codefromthecrypt]

cool. here's an example of a user accidentally tripping, though not sure it was related to Trivy or another scanner openzipkin/brave#1348

[codefromthecrypt]

I feel like the experience is still quite bad. I hope folks can reconsider after having some time to digest, and remove the "it" from the default expression. You can see it doesn't understand template variables

instrumentation/httpclient/src/it/httpclient_v43/pom.xml (pom)

Total: 4 (UNKNOWN: 0, LOW: 0, MEDIUM: 3, HIGH: 1, CRITICAL: 0)

┌──────────────────────────────────────┬────────────────┬──────────┬────────┬──────────────────────────┬───────────────┬────────────────────────────────────────────────────────────┐
│               Library                │ Vulnerability  │ Severity │ Status │    Installed Version     │ Fixed Version │                           Title                            │
├──────────────────────────────────────┼────────────────┼──────────┼────────┼──────────────────────────┼───────────────┼────────────────────────────────────────────────────────────┤
│ org.apache.httpcomponents:httpclient │ CVE-2012-6153  │ HIGH     │ fixed  │ @old-httpclient.version@ │ 4.2.3         │ SSL hostname verification bypass, incomplete CVE-2012-5783 │
│                                      │                │          │        │                          │               │ fix                                                        │
│                                      │                │          │        │                          │               │ https://avd.aquasec.com/nvd/cve-2012-6153                  │
│                                      ├────────────────┼──────────┤        │                          ├───────────────┼────────────────────────────────────────────────────────────┤
│                                      │ CVE-2014-3577  │ MEDIUM   │        │                          │ 4.3.5         │ Apache HttpComponents client / Apache CXF: SSL hostname    │
│                                      │                │          │        │                          │               │ verification bypass, incomplete CVE-2012-6153...           │
│                                      │                │          │        │                          │               │ https://avd.aquasec.com/nvd/cve-2014-3577                  │
│                                      ├────────────────┤          │        │                          ├───────────────┼────────────────────────────────────────────────────────────┤
│                                      │ CVE-2015-5262  │          │        │                          │ 4.3.6         │ jakarta-commons-httpclient, httpcomponents-core: missing   │
│                                      │                │          │        │                          │               │ HTTPS connection timeout                                   │
│                                      │                │          │        │                          │               │ https://avd.aquasec.com/nvd/cve-2015-5262                  │
│                                      ├────────────────┤          │        │                          ├───────────────┼────────────────────────────────────────────────────────────┤
│                                      │ CVE-2020-13956 │          │        │                          │ 4.5.13, 5.0.3 │ incorrect handling of malformed authority component in     │
│                                      │                │          │        │                          │               │ request URIs                                               │
│                                      │                │          │        │                          │               │ https://avd.aquasec.com/nvd/cve-2020-13956                 │
└──────────────────────────────────────┴────────────────┴──────────┴────────┴──────────────────────────┴───────────────┴────────────────────────────────────────────────────────────┘

[codefromthecrypt]

another example.. trivy is looking at files in the "target" directory which are definitely not source. This is just a side effect of the invoker plugin, used to run integration tests and needing a scratch area

instrumentation/kafka-clients/target/it/kafka1/pom.xml (pom)

Total: 6 (UNKNOWN: 0, LOW: 0, MEDIUM: 2, HIGH: 4, CRITICAL: 0)

┌─────────────────────────────────────┬────────────────┬──────────┬────────┬───────────────────┬───────────────┬─────────────────────────────────────────────────────────────┐
│               Library               │ Vulnerability  │ Severity │ Status │ Installed Version │ Fixed Version │                            Title                            │
├─────────────────────────────────────┼────────────────┼──────────┼────────┼───────────────────┼───────────────┼─────────────────────────────────────────────────────────────┤
│ org.apache.commons:commons-compress │ CVE-2024-25710 │ HIGH     │ fixed  │ 1.24.0            │ 1.26.0        │ commons-compress: Denial of service caused by an infinite   │
│                                     │                │          │        │                   │               │ loop for a corrupted...                                     │
│                                     │                │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2024-25710                  │
│                                     ├────────────────┤          │        │                   │               ├─────────────────────────────────────────────────────────────┤
│                                     │ CVE-2024-26308 │          │        │                   │               │ commons-compress: OutOfMemoryError unpacking broken Pack200 │
│                                     │                │          │        │                   │               │ file                                                        │
│                                     │                │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2024-26308                  │
├─────────────────────────────────────┼────────────────┤          │        ├───────────────────┼───────────────┼─────────────────────────────────────────────────────────────┤
│ org.xerial.snappy:snappy-java       │ CVE-2023-34455 │          │        │ 1.1.4             │ 1.1.10.1      │ snappy-java: Unchecked chunk length leads to DoS            │
│                                     │                │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2023-34455                  │
│                                     ├────────────────┤          │        │                   ├───────────────┼─────────────────────────────────────────────────────────────┤
│                                     │ CVE-2023-43642 │          │        │                   │ 1.1.10.4      │ snappy-java: Missing upper bound check on chunk length in   │
│                                     │                │          │        │                   │               │ snappy-java can lead...                                     │
│                                     │                │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2023-43642                  │
│                                     ├────────────────┼──────────┤        │                   ├───────────────┼─────────────────────────────────────────────────────────────┤
│                                     │ CVE-2023-34453 │ MEDIUM   │        │                   │ 1.1.10.1      │ snappy-java: Integer overflow in shuffle leads to DoS       │
│                                     │                │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2023-34453                  │
│                                     ├────────────────┤          │        │                   │               ├─────────────────────────────────────────────────────────────┤
│                                     │ CVE-2023-34454 │          │        │                   │               │ snappy-java: Integer overflow in compress leads to DoS      │
│                                     │                │          │        │                   │               │ https://avd.aquasec.com/nvd/cve-2023-34454                  │
└─────────────────────────────────────┴────────────────┴──────────┴────────┴───────────────────┴───────────────┴─────────────────────────────────────────────────────────────┘

[knqyf263]

Indeed the main issue is that src/it is 100pct not deployment code. it is a special phase that runs after a library is produced, so that you can do integration tests for reasons such as classpath isolation (it). There is no doubt this is a dev dependency, so if there is any way to classify this as a dev dep that's great!

Yes, devDependencies in Trivy means Packages that are only needed for local development and testing.
cf. https://docs.npmjs.com/specifying-dependencies-and-devdependencies-in-a-package-json-file

I believe this case is classified into testing dependencies. Therefore, we should hide them by default, but need to display them in some cases.

Independently, I would highly recommend never scanning ./target, unless you also scan directories like ./tmp and ./build. In other words, regardless of if there is an "it" in it or not, never scan target directories in a maven build.

It sounds reasonable, but let's start with src/it. Then, we can come back to this topic.

[codefromthecrypt]

agreed. I didn't know they were suppressed and somehow guessed there was some ranking going on. With the warning I wouldn't have mis-guessed, so good idea!

[codefromthecrypt]

It sounds reasonable, but let's start with src/it. Then, we can come back to this topic.

ok let's come back fast if the former is approved as otherwise you will get noise unless you have a clean checkout (due to side-effects of tests running)

[codefromthecrypt]

well, the generalized thing (anything in target) can be deferred as long as the "it" reclassification also applies to target, just seeing anything in target (say you enable that flag) would seem fishy

# after running a build, depending on config, an "it" pom is copied to the target directory
$ find . -name pom.xml |grep it
./context/log4j12/target/it/log4j12/pom.xml
./context/log4j12/src/it/log4j12/pom.xml
./brave/target/it/no_deps/pom.xml
./brave/src/it/no_deps/pom.xml
./instrumentation/servlet/target/it/servlet25/pom.xml
./instrumentation/servlet/src/it/servlet25/pom.xml
./instrumentation/httpclient/target/it/httpclient_v43/pom.xml
./instrumentation/httpclient/src/it/httpclient_v43/pom.xml
./instrumentation/kafka-clients/target/it/kafka1/pom.xml
./instrumentation/kafka-clients/src/it/kafka1/pom.xml
./instrumentation/grpc/target/it/grpc12/pom.xml
./instrumentation/grpc/src/it/grpc12/pom.xml
./instrumentation/jms-jakarta/target/it/jms30/pom.xml
./instrumentation/jms-jakarta/src/it/jms30/pom.xml
./instrumentation/spring-rabbit/pom.xml
./instrumentation/okhttp3/target/it/okhttp3_v3/pom.xml
./instrumentation/okhttp3/src/it/okhttp3_v3/pom.xml
./instrumentation/spring-web/target/it/spring3/pom.xml
./instrumentation/spring-web/src/it/spring3/pom.xml
./instrumentation/jms/target/it/jms11/pom.xml
./instrumentation/jms/src/it/jms11/pom.xml
./instrumentation/spring-webmvc/target/it/spring25/pom.xml
./instrumentation/spring-webmvc/target/it/servlet25/pom.xml
./instrumentation/spring-webmvc/src/it/spring25/pom.xml
./instrumentation/spring-webmvc/src/it/servlet25/pom.xml
./spring-beans/src/it/spring25/pom.xml

[knqyf263]

Ah, ok. **/it/*/pom.xml could be better. However, the directory name it is not that unique, so **/[src|target]/it/pom.xml may be less likely to cause problems. Thanks for sharing.