fix(license): stop spliting a long license text #7336

afdesk · 2024-08-13T10:18:29Z

Description

When we looks for licenses Trivy tries to split information about license through a regex.
but for some cases License field contains a long descriptive text.

This PR adds a detection of a long license text and keep it inside a new field - License, as Dmitriy suggested.

LinceseText field is available for JSON format only. for TABLE format Trivy shows CUSTOM License name instead of a long text.

For tests I use next image:

$ trivy i -d --cache-backend memory --scanners license saisk026/conda-test:v1

Before:

Afrer:

JSON output:

Related issues

Close don't split licenses from License field from python packaging #5204

Checklist

I've read the guidelines for contributing to this repository.
I've followed the conventions in the PR title.
I've added tests that prove my fix is effective or that my feature works.
I've updated the documentation with the relevant information (if needed).
I've added usage information (if the PR introduces new options)
I've included a "before" and "after" example to the description (if the PR is a user interface change).

afdesk · 2024-08-13T12:48:16Z

@knqyf263 i think it's ready for review.
please, take a look when you have time. thanks!

knqyf263 · 2024-08-14T03:23:45Z

Is there a way to distinguish between the license name and the license text?

afdesk · 2024-08-14T04:40:46Z

Is there a way to distinguish between the license name and the license text?

Right now, I'm not sure.
but I'll take a look and will show the cases.

knqyf263 · 2024-08-14T05:07:21Z

I want to show "unknown" for the license text.

afdesk · 2024-08-14T06:43:06Z

My concern is next.
Python deb packages can contain the license text inside license field:

cat /usr/share/doc/python3.9-minimal/copyright

but if I understand correctly it's a mistake.
the docs: First line (synopsis): an abbreviated name for the license.

afdesk · 2024-08-14T06:47:03Z

My concern is that there is no a correct way to distinguish between incorrect Permission to use, copy, modify, and distribute this software and its and correct license synopsis TinySCHEME or permissive for gpgv package.

afdesk · 2024-08-14T07:14:51Z

the same situation is with License field inside *.dist-info.METADATA.

some packages contain a correct license name (eg. Pympler): Apache License, Version 2.0.
some undefined license (eg zope): ZPL 2.1.
some packages contain the license text (eg menuinst):

afdesk · 2024-08-14T08:47:20Z

@knqyf263 I have an idea. trying

afdesk · 2024-08-14T08:56:26Z

I want to show "unknown" for the license text.

@knqyf263 Could you confirm that I understand correctly this requirement? thanks

knqyf263 · 2024-08-16T04:45:07Z

I know it's not ideal, but what if checking the length and the number of newlines?

func isLicenseName(license string) bool {
	// Check text length
	if len(license) < 100 {
		return true
	}

	// Count newlines
	if strings.Count(license, "\n") > 3 {
		return false
	}
        ...

knqyf263 · 2024-08-16T04:49:54Z

I want to show "unknown" for the license text.

@knqyf263 Could you confirm that I understand correctly this requirement? thanks

I don't want to show the license text there as it's too long. I thought we would show "UNKNOWN", but we know a license text. We just don't know the short name. How about "Custom"? Then, the license text can be stored in another field.

afdesk · 2024-08-19T04:54:15Z

I know it's not ideal, but what if checking the length and the number of newlines?

func isLicenseName(license string) bool {
	// Check text length
	if len(license) < 100 {
		return true
	}

	// Count newlines
	if strings.Count(license, "\n") > 3 {
		return false
	}
        ...

@knqyf263 I tried this way, you're right, it's not ideal.

Counting new lines don't affect on the output, because Trivy reads only one line from license in dpkg, python packages also contain a long single line license... so this check is always true.

About check text length. It works for long linceses in python.
but dpkg's licenses are still shown incorrectly.

i thought it's a long text, but actually it's a few first rows of several licenses:

License: Redistribution and use in source and binary forms, with or without
License: By obtaining, using, and/or copying this software and/or its
License: Permission to use, copy, modify, and distribute this software and
License: Redistribution and use in source and binary forms, with or without
License: This software is provided 'as-is', without any express or implied
License: Permission to use, copy, modify, and distribute this software and
License: Permission  is  hereby granted,  free  of charge,  to  any person
License: This software is provided 'as-is', without any express or implied
License: Permission is hereby granted, free of charge, to any person obtaining
   under the terms of the GNU General Public License as published by the
   section entitled ``GNU General Public License''.
License: Permission to use, copy, modify, and distribute this software and its
License: Permission to use, copy, modify, and distribute this software and its
License: This software is provided 'as-is', without any express or implied
License: Permission is hereby granted, free of charge, to any person obtaining
License: Redistribution and use in source and binary forms, with or without
License: This software is provided as-is, without express or implied
License: Permission to use, copy, modify, and distribute this software for any
License: Permission to use, copy, modify, and distribute this software and its
License:
License:  * Permission to use this software in any way is granted without
License: Permission to use, copy, modify, and distribute this software and its

afdesk · 2024-08-19T05:09:57Z

Right now, I can't see a good solution, but there are several options:

We just check text lengths, and don't update output for dpkg licenses, because it's a python's fault.
We keep a list of known licenses, and check every splited part.
if there is no matching, we print a license text.
We keep some keywords, and if we can find them inside a string, it'll mean a license text (or a part).
for example: as-is, redistribution, permission, using, use, @, http etc.

@knqyf263 wdyt?

afdesk · 2024-08-19T05:17:31Z

I want to show "unknown" for the license text.
I don't want to show the license text there as it's too long. I thought we would show "UNKNOWN", but we know a license text. We just don't know the short name. How about "Custom"? Then, the license text can be stored in another field.

You mean we should print CUSTOM license instead of a long license text for table output, and keep another field (ex licenseText) for JSON, right?
IMHO, It makes sense.
also I'd add a few words from license )

DmitriyLewen · 2024-08-19T06:00:56Z

i thought it's a long text, but actually it's a few first rows of several licenses:

What if we add one more check for copyright files:
if number of License: * fields more then one - split each line:

if 2 or more licenses are found in field - it is text license
if only 1 license is found in field - this is license name

e.g.
line 1 - text, line 2 - license name:

License: Redistribution and use in source and binary forms, with or without
License: BSD-3-Clause

My logic is as follows:
if you use multiple licenses separarted by and/or - you will use only one License: * field.
But if you use multiple License: * fields:

you will use one license for each License: * field.
e.g.:
```
License: MIT
License: BSD-3-Clause
```
in other cases it is license text.

afdesk · 2024-08-19T06:19:34Z

@DmitriyLewen that's an interesting idea.

there are next cases for perl and python packages:

perl:

License: GPL-1+ or Artistic or Artistic-dist

python3.9:

License: This software is provided 'as-is', without any express or implied

i'm not sure we can separate these cases, but maybe if we also will check string length...

a long string with a few splited licenses is a text.
what is long? I think it about 50 chars.
wdyt?

DmitriyLewen · 2024-08-19T07:15:33Z

but maybe if we also will check string length...

Yeah. That's what I thought
if there's only one field - we check the length and number of lines.

I think it about 50 chars.

What if use 30 characters + no saved licenses found

knqyf263 · 2024-08-21T02:56:40Z

You mean we should print CUSTOM license instead of a long license text for table output, and keep another field (ex licenseText) for JSON, right?

Exactly

afdesk · 2024-08-23T04:19:32Z

@knqyf263 @DmitriyLewen
I tried severel ways to separate a long license text from a license name, and the best result for me is a detection by keywords.

There were selected a few obvious words, that can appear inside license texts only.

Please, take a look at this suggestion when you have free time. thanks!

DmitriyLewen

@afdesk I left comments. Take a look, please.

I tried severel ways to separate a long license text from a license name, and the best result for me is a detection by keywords.

Can you give examples of problems for other methods to save time if we need to come back to this question?

DmitriyLewen · 2024-08-23T06:39:43Z

pkg/scanner/local/scan.go

+					Severity:    dbTypes.SeverityUnknown.String(),
+					Category:    ftypes.CategoryUnknown,
+					PkgName:     pkg.Name,
+					Name:        "CUSTOM License",


We need to add info when we use CUSTOM License in docs and license Name/log message

We might want to add the first few words of the license text to the name.

We might want to add the first few words of the license text to the name.

if @knqyf263 agrees too, I'll add it.
I thought about filtering by CUSTOM License, but maybe it doesn't matter

@DmitriyLewen ))

I thought about CUSTOM License again. This may not be clear to users.
What if we use the name Incomparable License/Unmatched License
This means that we can't compare license text with known Trivy licenses and therefore store license as text.
cc. @knqyf263

I saw some places calling them "custom licenses", so I don't think it's that bad.

Custom licenses are strongly disfavored and should be used only in extraordinary circumstances. An Agency ought to adopt a license approved by the Open Source Initiative (OSI) at opensource.org.

https://opensource.org/authority

If you are using a license that hasn't been assigned an SPDX identifier, or if you are using a custom license, use a string value like this one:

https://docs.npmjs.com/cli/v10/configuring-npm/package-json#license

Alternatively, you can use a LicenseRef- custom license identifier to refer to a license that is not on the SPDX License List, such as the following:

https://spdx.github.io/spdx-spec/v2.3/using-SPDX-short-identifiers-in-source-files/

Or "non-standard" or something like that.

Incomparable License/Unmatched License are also unclear what these did not match.

pkg/types/license.go

DmitriyLewen · 2024-08-23T06:43:25Z

pkg/licensing/normalize.go

 func SplitLicenses(str string) []string {
 	if str == "" {
 		return nil
 	}
+	if isLicenseText(strings.ToLower(str)) {
+		return []string{
+			"text://" + str,


We use file:// prefix in Python.
What if we create constants for these prefixes?

afdesk · 2024-08-23T07:21:07Z

I tried severel ways to separate a long license text from a license name, and the best result for me is a detection by keywords.

Can you give examples of problems for other methods to save time if we need to come back to this question?

The main problem is to separate a license name and a license text.
Some developers put a full license text inside a license field, where should be a license name (or a license ID).

Checking the length doesn't work because there are correct long lincenses (it's already added to test cases):

Common Development and Distribution License 1.0 (CDDL-1.0)

the number of newlines doesn't work too, because we read only one line from copyright or license files.

the number of spaces doesn't work, because there are too long correct license names (ex CDDL-1.0).

saisk026/conda-test:v1 is a good image for license testing.

DmitriyLewen

LGTM

@knqyf263 wdyt about this way?

DmitriyLewen · 2024-08-26T11:02:40Z

rpc/cache/service.pb.go

@@ -1,6 +1,6 @@
 // Code generated by protoc-gen-go. DO NOT EDIT.
 // versions:
-// 	protoc-gen-go v1.27.1
+// 	protoc-gen-go v1.34.0


looks like we can skip this change

this change results the versions of protoc-gen-go for cache/ and common to the same version. it was build automatically

removed this update for this PR)

knqyf263 · 2024-09-03T09:29:39Z

I tried severel ways to separate a long license text from a license name, and the best result for me is a detection by keywords.

OK, we don't have an easy way. Let's see how it goes.

afdesk · 2024-09-03T10:16:03Z

I tried severel ways to separate a long license text from a license name, and the best result for me is a detection by keywords.

OK, we don't have an easy way. Let's see how it goes.

Should i fix something to add this PR in 0.55?

knqyf263 · 2024-09-03T10:38:22Z

Should i fix something to add this PR in 0.55?

I'm reviewing the changes now. I'll update you soon.
If we don't make it for v0.55.0, we can include it in v0.55.1.

Signed-off-by: knqyf263 <[email protected]>

knqyf263

I refactored but realized there was no test. I'm not sure my changes work as expected.
@afdesk Could you add a test for license texts?

knqyf263 · 2024-09-04T06:05:47Z

pkg/types/license.go

@@ -22,6 +22,9 @@ type DetectedLicense struct {
 	// Name holds a detected license name
 	Name string

+	// LicenseText holds a long license text if Trivy detects a license name as a license text
+	LicenseText string


nit: Since this is "DetectedLicense", Text looks enough.

Suggested change

LicenseText string

Text string

knqyf263 · 2024-09-04T06:05:53Z

rpc/common/service.proto

+  string               name         = 5;
+  float                confidence   = 6;
+  string               link         = 7;
+  string               license_text = 8;


afdesk · 2024-09-04T10:02:48Z

but realized there was no test. I'm not sure my changes work as expected. @afdesk Could you add a test for license texts?

yeh, sure. the test is added

afdesk added 3 commits August 13, 2024 16:17

fix(license): handling unknown licenses

08ad65a

update licenses for os packages

6bbe481

fix: accumulate unknown licenses

e8a9288

afdesk changed the title ~~fix(license): handling unknown licenses~~ fix(license): combining unknown licenses Aug 13, 2024

fix: golinter ruleguard

e6090fa

afdesk marked this pull request as ready for review August 13, 2024 12:30

afdesk requested review from knqyf263 and DmitriyLewen as code owners August 13, 2024 12:30

feat: add check a long license text

a87facc

afdesk marked this pull request as draft August 23, 2024 03:46

add license text detection

87d93b9

afdesk marked this pull request as ready for review August 23, 2024 04:39

DmitriyLewen reviewed Aug 23, 2024

View reviewed changes

afdesk added 5 commits August 26, 2024 12:41

add rpc fields

223a8b5

add test cases for prc

fb3e407

fix: using the existed test case

a702a64

add constants for license prefixes

aa23f3e

trim license text

65f2637

afdesk changed the title ~~fix(license): combining unknown licenses~~ fix(license): stop spliting a long license text Aug 26, 2024

afdesk requested a review from DmitriyLewen August 26, 2024 08:28

DmitriyLewen reviewed Aug 26, 2024

View reviewed changes

remove version update

0ad0924

afdesk requested a review from DmitriyLewen August 26, 2024 11:25

refactor

e3a1950

Signed-off-by: knqyf263 <[email protected]>

knqyf263 reviewed Sep 4, 2024

View reviewed changes

afdesk added 2 commits September 4, 2024 15:03

refactor: rename fields

bd9aa76

add test case

291d352

afdesk added 3 commits September 4, 2024 16:15

test: add OS Package

5f63241

update protoc

398c3df

test: update protoc tests

af88f47

afdesk requested a review from knqyf263 September 4, 2024 10:48

knqyf263 approved these changes Sep 5, 2024

View reviewed changes

knqyf263 added this pull request to the merge queue Sep 5, 2024

Merged via the queue into aquasecurity:main with commit 4926da7 Sep 5, 2024
12 checks passed

aqua-bot mentioned this pull request Sep 5, 2024

release: v0.56.0 [main] #7447

Open

fix(license): stop spliting a long license text #7336

fix(license): stop spliting a long license text #7336

Conversation

afdesk commented Aug 13, 2024 • edited Loading

Description

Related issues

Checklist

afdesk commented Aug 13, 2024

knqyf263 commented Aug 14, 2024

afdesk commented Aug 14, 2024

knqyf263 commented Aug 14, 2024

afdesk commented Aug 14, 2024 • edited Loading

afdesk commented Aug 14, 2024 • edited Loading

afdesk commented Aug 14, 2024 • edited Loading

afdesk commented Aug 14, 2024

afdesk commented Aug 14, 2024 • edited Loading

knqyf263 commented Aug 16, 2024

knqyf263 commented Aug 16, 2024

afdesk commented Aug 19, 2024

afdesk commented Aug 19, 2024

afdesk commented Aug 19, 2024

DmitriyLewen commented Aug 19, 2024

afdesk commented Aug 19, 2024 • edited Loading

DmitriyLewen commented Aug 19, 2024

knqyf263 commented Aug 21, 2024

afdesk commented Aug 23, 2024 • edited Loading

DmitriyLewen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afdesk commented Aug 23, 2024

DmitriyLewen left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

knqyf263 commented Sep 3, 2024

afdesk commented Sep 3, 2024

knqyf263 commented Sep 3, 2024

knqyf263 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

afdesk commented Sep 4, 2024

afdesk commented Aug 13, 2024 •

edited

Loading

afdesk commented Aug 14, 2024 •

edited

Loading

afdesk commented Aug 14, 2024 •

edited

Loading

afdesk commented Aug 14, 2024 •

edited

Loading

afdesk commented Aug 14, 2024 •

edited

Loading

afdesk commented Aug 19, 2024 •

edited

Loading

afdesk commented Aug 23, 2024 •

edited

Loading

DmitriyLewen left a comment •

edited

Loading