search

CycloneDX / cyclonedx-cocoapods

Creates CycloneDX Software Bill-of-Materials (SBOM) from Objective-C and Swift projects that use CocoaPods.
Apache License 2.0
21 stars 12 forks source link

Fix length of "author" and "publisher" in order to upload to DT #65

Closed sev-hack closed 8 months ago

sev-hack commented 9 months ago

Hi there!

I noticed that Dependency Track (DT) doesn't analyze the BOM with length of "author" and "publisher" fields more than 255 characters. See the log file below:

2024-01-16 10:14:04,206 ERROR [BomUploadProcessingTask] Error while processing bom
javax.jdo.JDOFatalUserException: Attempt to store value "Mark J. Cox <mark@openssl.org>, Ralf S. Engelschall <rse@openssl.org>, Dr. Stephen Henson <steve@openssl.org>, Ben Laurie <ben@openssl.org>, Lutz Jänicke <jaenicke@openssl.org>, Nils Larsch <nils@openssl.org>, Richard Levitte <nils@openssl.org>, Bodo Möller <bodo@openssl.org>, Ulf Möller <ulf@openssl.org>, Andy Polyakov <appro@openssl.org>, Geoff Thorpe <geoff@openssl.org>, Holger Reif <holger@openssl.org>, Paul C. Sutton <geoff@openssl.org>, Eric A. Young <eay@cryptsoft.com>, Tim Hudson <tjh@cryptsoft.com>, Justin Plouffe <plouffe.justin@gmail.com>" in column ""PUBLISHER"" that has maximum length of 255. Please correct your data!
at org.datanucleus.api.jdo.JDOAdapter.getJDOExceptionForNucleusException(JDOAdapter.java:678)
at org.datanucleus.api.jdo.JDOPersistenceManager.jdoMakePersistent(JDOPersistenceManager.java:702)
at org.datanucleus.api.jdo.JDOPersistenceManager.makePersistent(JDOPersistenceManager.java:722)
at alpine.persistence.AbstractAlpineQueryManager.persist(AbstractAlpineQueryManager.java:427)
at org.dependencytrack.persistence.ComponentQueryManager.createComponent(ComponentQueryManager.java:306)
at org.dependencytrack.persistence.QueryManager.createComponent(QueryManager.java:496)
at org.dependencytrack.tasks.BomUploadProcessingTask.processComponent(BomUploadProcessingTask.java:183)
at org.dependencytrack.tasks.BomUploadProcessingTask.inform(BomUploadProcessingTask.java:128)
at alpine.event.framework.BaseEventService.lambda$publish$0(BaseEventService.java:101)
at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(Unknown Source)
at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(Unknown Source)
at java.base/java.lang.Thread.run(Unknown Source)

According to my internal tests, small changes in the file https://github.com/CycloneDX/cyclonedx-cocoapods/blob/main/lib/cyclonedx/cocoapods/bom_builder.rb on line 71 and 72 helped to put in BOM only first 250 characters:

xml.author author.slice(0, 250) unless author.nil?
xml.publisher author.slice(0, 250) unless author.nil?

This problem sounds similar to these issues in DT repository:

@macblazer Is it possible to add this fix to the next release?

macblazer commented 9 months ago

We could add a CLI flag to trim the length of the publisher field. The last DT issue you linked appears to have been fixed in early 2023 for the component author field at least.

Are you running the latest version of Dependency Track? I don't really want to add code to workaround problems in older versions of DT, but if the problem still persists today then maybe it's worthwhile to deal with it here.

If we do add a flag here, do you think the default behavior should be to trim strings or to leave them alone? Adding the CLI flag would change to the non-default behavior. I lean towards having the full text be the default, and the flag to shorten them being --shortened-strings or maybe --dt-compatible-string-length or similar.

sev-hack commented 9 months ago

Hello @macblazer. Thanks for your reply.

Tested on both 4.7.1 and the latest 4.10.1 DT versions - the same results.

Agree with you - the default behavior should be to leave the string alone. In my opinion --shortened-strings will be the best CLI-flag.

Just for your information, I noticed almost the same bug but with the "PURL" field when used cyclonedx-node-npm - in this case there is a short-PURLs (see https://github.com/CycloneDX/cyclonedx-node-npm#usage) flag that can be useful. So I think not only "Author" and "Publisher" fields can exceed 255 characters length, but also "PURL" and other fields

macblazer commented 9 months ago

Yeah, I clicked into all those Dependency Track issues you linked and read through them. It definitely seems to be some DT database specific problems where the DT developers assumed these text fields would be 255 characters or fewer.

Cutting off the PURL anywhere seems like a terrible idea since that's the unique thing that identifies the component, but if DT can't handle it I suppose we could add a call to .slice() everywhere that is output also. Ick.

macblazer commented 9 months ago

DT seems to be aware of this widespread issue in their database schema and code. See DependencyTrack/dependency-track#2076 for some more discussion.

sev-hack commented 9 months ago

Yeah, thanks for suggestion.

So what will we do in this situation - wait for the fix from DT team (can be time consuming) or fix it from our side?

macblazer commented 9 months ago

I'm ok with adding a flag here so people can get things done soon. If you want to open a PR, @sev-hack, I can review and merge.

If left to me I probably won't be able to get to any coding today, but likely this weekend some time.

sev-hack commented 9 months ago

Since I'm not good at Ruby programming, I'd really appreciate it if you'd do that.

That's not urgent task so take your time.

sev-hack commented 9 months ago

Hello, @macblazer! Any updates?

macblazer commented 9 months ago

Yep, made good progress this past weekend. I will cleanup the code today and get a PR ready for it.

I settled on adding a CLI flag --shortened-strings length where the caller can specify what length they want things trimmed to. The code trims the author, publisher, and purl fields to the specified length.

If people find other fields that we output also need to be trimmed, we can add those.