diff --git a/internal/spdxlicense/license.go b/internal/spdxlicense/license.go index ff8bd1265cd..b126c0e7fe9 100644 --- a/internal/spdxlicense/license.go +++ b/internal/spdxlicense/license.go @@ -62,3 +62,30 @@ func stripScheme(url string) string { url = strings.TrimPrefix(url, "http://") return url } + +// HasTopLevelOr reports whether the expression has an OR operator outside of every parenthesised +// group. +// +// It matters when an expression is joined onto others with AND, because SPDX applies AND before +// OR: joining "MIT OR (Apache-2.0 AND BSD-3-Clause)" unwrapped gives +// "MIT OR (Apache-2.0 AND BSD-3-Clause) AND GPL-3.0-only", which reads as +// "MIT OR ((Apache-2.0 AND BSD-3-Clause) AND GPL-3.0-only)" and drops GPL-3.0-only from the first +// choice. Testing only the first and last character misses that, and misses +// "(MIT OR Apache-2.0) OR (GPL-3.0-only AND MIT)" as well. +func HasTopLevelOr(expression string) bool { + depth := 0 + for i, c := range expression { + switch c { + case '(': + depth++ + case ')': + depth-- + default: + if depth <= 0 && strings.HasPrefix(expression[i:], " OR ") { + return true + } + } + } + + return false +} diff --git a/internal/spdxlicense/license_test.go b/internal/spdxlicense/license_test.go index e1444a62c6c..4685ab6ab11 100644 --- a/internal/spdxlicense/license_test.go +++ b/internal/spdxlicense/license_test.go @@ -83,3 +83,28 @@ func TestSPDXIDRecognition(t *testing.T) { }) } } + +func TestHasTopLevelOr(t *testing.T) { + tests := []struct { + expression string + want bool + }{ + {"", false}, + {"MIT", false}, + {"MIT AND Apache-2.0", false}, + {"Apache-2.0 WITH LLVM-exception", false}, + {"MIT OR Apache-2.0", true}, + {"(MIT OR Apache-2.0)", false}, + {"ISC AND (BSD-3-Clause OR MIT)", false}, + {"MIT OR (Apache-2.0 AND BSD-3-Clause)", true}, + {"(GPL-2.0-only WITH Linux-syscall-note) OR MIT", true}, + {"(MIT OR Apache-2.0) OR (GPL-3.0-only AND LGPL-2.1-only)", true}, + {"(MIT AND (Apache-2.0 OR BSD-3-Clause))", false}, + {"LicenseRef-one-thing-first", false}, + } + for _, tt := range tests { + t.Run(tt.expression, func(t *testing.T) { + assert.Equal(t, tt.want, HasTopLevelOr(tt.expression)) + }) + } +} diff --git a/syft/format/internal/cyclonedxutil/helpers/licenses.go b/syft/format/internal/cyclonedxutil/helpers/licenses.go index 3330c86a7f4..45598817796 100644 --- a/syft/format/internal/cyclonedxutil/helpers/licenses.go +++ b/syft/format/internal/cyclonedxutil/helpers/licenses.go @@ -194,8 +194,11 @@ func processLicenseURLs(l pkg.License, spdxID string, populate *cyclonedx.Licens func mergeSPDX(ex []string) string { var candidate []string for _, e := range ex { - // if the expression does not have balanced parens add them - if !strings.HasPrefix(e, "(") && !strings.HasSuffix(e, ")") { + // wrap when the expression does not have balanced parens, and also when it carries an OR + // outside of any parens: SPDX applies AND before OR, so joining + // "MIT OR (Apache-2.0 AND BSD-3-Clause)" as-is changes what it means, and the + // prefix/suffix check on its own does not catch that + if spdxlicense.HasTopLevelOr(e) || (!strings.HasPrefix(e, "(") && !strings.HasSuffix(e, ")")) { e = "(" + e + ")" } candidate = append(candidate, e) diff --git a/syft/format/internal/cyclonedxutil/helpers/licenses_test.go b/syft/format/internal/cyclonedxutil/helpers/licenses_test.go index 257e96acb47..5232579a717 100644 --- a/syft/format/internal/cyclonedxutil/helpers/licenses_test.go +++ b/syft/format/internal/cyclonedxutil/helpers/licenses_test.go @@ -314,3 +314,40 @@ func TestDecodeLicenses(t *testing.T) { }) } } + +func Test_mergeSPDX(t *testing.T) { + tests := []struct { + name string + input []string + expected string + }{ + { + // without the parens this reads as + // "((GPL-3.0-only AND MIT-0) AND MIT) OR (Apache-2.0 AND BSD-3-Clause)", + // so Apache-2.0 plus BSD-3-Clause alone would satisfy it + name: "expression with a top-level OR is wrapped", + input: []string{"MIT OR (Apache-2.0 AND BSD-3-Clause)", "GPL-3.0-only AND MIT-0"}, + expected: "(MIT OR (Apache-2.0 AND BSD-3-Clause)) AND (GPL-3.0-only AND MIT-0)", + }, + { + name: "top-level AND keeps the text it has today", + input: []string{"ISC AND (BSD-3-Clause OR MIT)", "GPL-3.0-only AND MIT-0"}, + expected: "ISC AND (BSD-3-Clause OR MIT) AND (GPL-3.0-only AND MIT-0)", + }, + { + name: "single expression is unwrapped by reduceOuter", + input: []string{"MIT OR Apache-2.0"}, + expected: "MIT OR Apache-2.0", + }, + { + name: "plain ids", + input: []string{"MIT", "Apache-2.0"}, + expected: "(MIT) AND (Apache-2.0)", + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + assert.Equal(t, test.expected, mergeSPDX(test.input)) + }) + } +} diff --git a/syft/format/internal/spdxutil/helpers/license.go b/syft/format/internal/spdxutil/helpers/license.go index 8ba477a6af0..27d80a58b9d 100644 --- a/syft/format/internal/spdxutil/helpers/license.go +++ b/syft/format/internal/spdxutil/helpers/license.go @@ -42,15 +42,8 @@ func joinLicenses(licenses []SPDXLicense) string { for _, l := range licenses { v := l.ID - // check if license does not start or end with parens - if !strings.HasPrefix(v, "(") && !strings.HasSuffix(v, ")") { - // if license contains AND, OR, or WITH, then wrap in parens - if strings.Contains(v, " AND ") || - strings.Contains(v, " OR ") || - strings.Contains(v, " WITH ") { - newLicenses = append(newLicenses, "("+v+")") - continue - } + if needsParens(v) { + v = "(" + v + ")" } newLicenses = append(newLicenses, v) } @@ -58,6 +51,24 @@ func joinLicenses(licenses []SPDXLicense) string { return strings.Join(newLicenses, " AND ") } +// needsParens reports whether an expression has to be wrapped before it is joined onto the others +// with AND. An expression carrying a top-level OR has to be, or the join changes what it means. +// The rest is the rule this used before, so expressions that are already emitted correctly keep +// the text they have today. +func needsParens(expression string) bool { + if spdxlicense.HasTopLevelOr(expression) { + return true + } + + if strings.HasPrefix(expression, "(") || strings.HasSuffix(expression, ")") { + return false + } + + return strings.Contains(expression, " AND ") || + strings.Contains(expression, " OR ") || + strings.Contains(expression, " WITH ") +} + type SPDXLicense struct { // Valid SPDX ID OR License Value (should have LicenseRef- prefix and be sanitized) // OR combination of the above as a valid SPDX License Expression as defined in Annex D. diff --git a/syft/format/internal/spdxutil/helpers/license_test.go b/syft/format/internal/spdxutil/helpers/license_test.go index 4dd262ec3d8..99e0a98d4d1 100644 --- a/syft/format/internal/spdxutil/helpers/license_test.go +++ b/syft/format/internal/spdxutil/helpers/license_test.go @@ -170,6 +170,46 @@ func Test_joinLicenses(t *testing.T) { args: []SPDXLicense{{ID: "MIT AND Apache"}, {ID: "GPL-3.0-only"}}, want: "(MIT AND Apache) AND GPL-3.0-only", }, + { + // without the parens this reads as "MIT OR (Apache-2.0 AND BSD-3-Clause AND GPL-3.0-only)", + // which allows MIT on its own and drops the GPL-3.0-only obligation + name: "expression ending in a paren is still wrapped", + args: []SPDXLicense{{ID: "MIT OR (Apache-2.0 AND BSD-3-Clause)"}, {ID: "GPL-3.0-only"}}, + want: "(MIT OR (Apache-2.0 AND BSD-3-Clause)) AND GPL-3.0-only", + }, + { + name: "expression starting with a paren is still wrapped", + args: []SPDXLicense{{ID: "(MIT OR Apache-2.0) OR GPL-3.0-only"}, {ID: "LGPL-2.1-only"}}, + want: "((MIT OR Apache-2.0) OR GPL-3.0-only) AND LGPL-2.1-only", + }, + { + name: "separate groups joined at the top level are wrapped", + args: []SPDXLicense{{ID: "(MIT OR Apache-2.0) OR (GPL-3.0-only AND LGPL-2.1-only)"}, {ID: "GPL-3.0-only"}}, + want: "((MIT OR Apache-2.0) OR (GPL-3.0-only AND LGPL-2.1-only)) AND GPL-3.0-only", + }, + { + name: "a single parenthesised group is not wrapped again", + args: []SPDXLicense{{ID: "(MIT OR Apache-2.0)"}, {ID: "GPL-3.0-only"}}, + want: "(MIT OR Apache-2.0) AND GPL-3.0-only", + }, + { + // top-level operator is AND, so joining is already safe and the text must not move + name: "expression whose top-level operator is AND keeps its text", + args: []SPDXLicense{{ID: "ISC AND (BSD-3-Clause OR MIT)"}, {ID: "MIT"}}, + want: "ISC AND (BSD-3-Clause OR MIT) AND MIT", + }, + { + // nothing is joined onto it, so the parens are redundant, but a lone compound + // expression is already wrapped today and this keeps the two shapes consistent + name: "single expression with a top-level OR is wrapped", + args: []SPDXLicense{{ID: "(GPL-2.0-only WITH Linux-syscall-note) OR MIT"}}, + want: "((GPL-2.0-only WITH Linux-syscall-note) OR MIT)", + }, + { + name: "single expression without parens keeps the wrapping it has today", + args: []SPDXLicense{{ID: "MIT OR Apache-2.0"}}, + want: "(MIT OR Apache-2.0)", + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) {