Skip to content

Commit a642a6c

Browse files
committed
♻️ feat: treat multiple gemspec licenses as 'AND' relationship
- When multiple licenses are specified in a gemspec, we now assume all licenses apply (AND) rather than alternatives (OR). - This is a safer, more conservative approach. Users can override this in the configuration if the intention was OR. - Added tests for multiple licenses (both non-conflicting and potentially conflicting) to verify they are joined with "AND".
1 parent 3625d88 commit a642a6c

File tree

3 files changed

+117
-40
lines changed

3 files changed

+117
-40
lines changed

pkg/deps/gemspec_test.go

Lines changed: 67 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
package deps
1919

2020
import (
21+
"fmt"
2122
"os"
2223
"path/filepath"
2324
"testing"
@@ -32,8 +33,7 @@ const (
3233
func TestRubyGemspecResolver(t *testing.T) {
3334
resolver := new(GemspecResolver)
3435

35-
// toml-merge case: parse gemspec, detect license and dependencies
36-
{
36+
t.Run("toml-merge case", func(t *testing.T) {
3737
tmp := t.TempDir()
3838
if err := copyRuby("testdata/ruby/toml-merge", tmp); err != nil {
3939
t.Fatal(err)
@@ -65,10 +65,9 @@ func TestRubyGemspecResolver(t *testing.T) {
6565
if !found {
6666
t.Errorf("expected toml-rb dependency, got %v", report.Resolved)
6767
}
68-
}
68+
})
6969

70-
// citrus case: transitive dependency resolution via installed gems
71-
{
70+
t.Run("citrus case", func(t *testing.T) {
7271
tmp := t.TempDir()
7372
gemHome := filepath.Join(tmp, "gemhome")
7473
specsDir := filepath.Join(gemHome, "specifications")
@@ -149,5 +148,68 @@ end
149148
t.Errorf("expected citrus license MIT, got %s", license)
150149
}
151150
}
151+
})
152+
153+
t.Run("multiple licenses case (non-conflicting)", func(t *testing.T) {
154+
testMultiLicense(t, resolver, "multi-license", "['MIT', 'Apache-2.0']", "MIT AND Apache-2.0")
155+
})
156+
157+
t.Run("multiple licenses case (conflicting/incompatible)", func(t *testing.T) {
158+
testMultiLicense(t, resolver, "conflicting-license", "['GPL-2.0', 'Apache-2.0']", "GPL-2.0 AND Apache-2.0")
159+
})
160+
}
161+
162+
func testMultiLicense(t *testing.T, resolver *GemspecResolver, gemName, licensesStr, expectedLicense string) {
163+
tmp := t.TempDir()
164+
gemHome := filepath.Join(tmp, "gemhome")
165+
specsDir := filepath.Join(gemHome, "specifications")
166+
if err := os.MkdirAll(specsDir, 0o755); err != nil {
167+
t.Fatal(err)
168+
}
169+
t.Setenv("GEM_HOME", gemHome)
170+
171+
// Create multi-license gem
172+
gemContent := fmt.Sprintf(`
173+
Gem::Specification.new do |s|
174+
s.name = '%s'
175+
s.version = '1.0.0'
176+
s.licenses = %s
177+
end
178+
`, gemName, licensesStr)
179+
if err := writeFileRuby(filepath.Join(specsDir, fmt.Sprintf("%s-1.0.0.gemspec", gemName)), gemContent); err != nil {
180+
t.Fatal(err)
181+
}
182+
183+
// Create project gemspec
184+
projectContent := fmt.Sprintf(`
185+
Gem::Specification.new do |s|
186+
s.name = 'project'
187+
s.version = '0.0.1'
188+
s.add_dependency '%s', '~> 1.0'
189+
end
190+
`, gemName)
191+
gemspec := filepath.Join(tmp, "project.gemspec")
192+
if err := writeFileRuby(gemspec, projectContent); err != nil {
193+
t.Fatal(err)
194+
}
195+
196+
cfg := &ConfigDeps{Files: []string{gemspec}}
197+
report := Report{}
198+
if err := resolver.Resolve(gemspec, cfg, &report); err != nil {
199+
t.Fatal(err)
200+
}
201+
202+
found := false
203+
for _, r := range report.Resolved {
204+
if r.Dependency == gemName {
205+
found = true
206+
if r.LicenseSpdxID != expectedLicense {
207+
t.Errorf("expected %s license '%s', got '%s'", gemName, expectedLicense, r.LicenseSpdxID)
208+
}
209+
break
210+
}
211+
}
212+
if !found {
213+
t.Errorf("expected %s dependency", gemName)
152214
}
153215
}

pkg/deps/ruby.go

Lines changed: 44 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -160,10 +160,43 @@ func (r *GemspecResolver) CanResolve(file string) bool {
160160
// them along with their transitive dependencies. It reports the found dependencies
161161
// and their licenses.
162162
func (r *GemspecResolver) Resolve(file string, config *ConfigDeps, report *Report) error {
163-
f, err := os.Open(file)
163+
deps, err := parseInitialDependencies(file)
164164
if err != nil {
165165
return err
166166
}
167+
168+
if errResolve := resolveTransitiveDependencies(deps); errResolve != nil {
169+
return errResolve
170+
}
171+
172+
for name, version := range deps {
173+
if exclude, _ := config.IsExcluded(name, version); exclude {
174+
continue
175+
}
176+
if l, ok := config.GetUserConfiguredLicense(name, version); ok {
177+
report.Resolve(&Result{Dependency: name, LicenseSpdxID: l, Version: version})
178+
continue
179+
}
180+
181+
// Check installed gems first, then fallback to RubyGems API
182+
licenseID := fetchInstalledLicense(name, version)
183+
if licenseID == "" {
184+
licenseID, err = fetchRubyGemsLicense(name, version)
185+
}
186+
if err != nil || licenseID == "" {
187+
report.Skip(&Result{Dependency: name, LicenseSpdxID: Unknown, Version: version})
188+
continue
189+
}
190+
report.Resolve(&Result{Dependency: name, LicenseSpdxID: licenseID, Version: version})
191+
}
192+
return nil
193+
}
194+
195+
func parseInitialDependencies(file string) (map[string]string, error) {
196+
f, err := os.Open(file)
197+
if err != nil {
198+
return nil, err
199+
}
167200
defer f.Close()
168201

169202
scanner := bufio.NewScanner(f)
@@ -181,10 +214,12 @@ func (r *GemspecResolver) Resolve(file string, config *ConfigDeps, report *Repor
181214
}
182215
}
183216
if err := scanner.Err(); err != nil {
184-
return err
217+
return nil, err
185218
}
219+
return deps, nil
220+
}
186221

187-
// Recursive resolution
222+
func resolveTransitiveDependencies(deps map[string]string) error {
188223
queue := make([]string, 0, len(deps))
189224
visited := make(map[string]struct{}, len(deps))
190225
for name := range deps {
@@ -214,7 +249,8 @@ func (r *GemspecResolver) Resolve(file string, config *ConfigDeps, report *Repor
214249
for _, dep := range newDeps {
215250
if _, ok := visited[dep]; !ok {
216251
if len(queue) >= 10000 {
217-
return fmt.Errorf("dependency graph exceeded maximum size of 10000 nodes (current: %d). This may indicate a circular dependency or an unusually large dependency tree.", len(queue))
252+
return fmt.Errorf("dependency graph exceeded maximum size of 10000 nodes (current: %d). "+
253+
"This may indicate a circular dependency or an unusually large dependency tree", len(queue))
218254
}
219255
visited[dep] = struct{}{}
220256
queue = append(queue, dep)
@@ -224,28 +260,6 @@ func (r *GemspecResolver) Resolve(file string, config *ConfigDeps, report *Repor
224260
}
225261
}
226262
}
227-
228-
for name, version := range deps {
229-
if exclude, _ := config.IsExcluded(name, version); exclude {
230-
continue
231-
}
232-
if l, ok := config.GetUserConfiguredLicense(name, version); ok {
233-
report.Resolve(&Result{Dependency: name, LicenseSpdxID: l, Version: version})
234-
continue
235-
}
236-
237-
// Check installed gems first, then fallback to RubyGems API
238-
licenseID := fetchInstalledLicense(name, version)
239-
var err error
240-
if licenseID == "" {
241-
licenseID, err = fetchRubyGemsLicense(name, version)
242-
}
243-
if err != nil || licenseID == "" {
244-
report.Skip(&Result{Dependency: name, LicenseSpdxID: Unknown, Version: version})
245-
continue
246-
}
247-
report.Resolve(&Result{Dependency: name, LicenseSpdxID: licenseID, Version: version})
248-
}
249263
return nil
250264
}
251265

@@ -613,16 +627,16 @@ func parseGemspecInfo(path string) (gemName, gemLicense string, err error) {
613627
}
614628
if len(licenses) > 0 {
615629
// NOTE: When multiple licenses are declared in the gemspec, we assume they are
616-
// alternatives and represent them with SPDX-style "OR". Some gems may instead
617-
// intend all listed licenses to apply ("AND"), which is not distinguished here.
630+
// all required ("AND") to be conservative. If the author intended "OR",
631+
// the user can override this in the configuration.
618632
if len(licenses) > 1 {
619633
gemRef := name
620634
if gemRef == "" {
621635
gemRef = path
622636
}
623-
logger.Log.Warnf("Multiple licenses found for gem %s: %v. Assuming 'OR' relationship.", gemRef, licenses)
637+
logger.Log.Warnf("Multiple licenses found for gem %s: %v. Assuming 'AND' relationship for safety.", gemRef, licenses)
624638
}
625-
license = strings.Join(licenses, " OR ")
639+
license = strings.Join(licenses, " AND ")
626640
}
627641
}
628642
}

pkg/deps/ruby_test.go

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -358,7 +358,7 @@ func TestGemspecIgnoresCommentedRuntimeDependencies(t *testing.T) {
358358
func TestFetchInstalledLicense(t *testing.T) {
359359
gemHome := t.TempDir()
360360
specsDir := filepath.Join(gemHome, "specifications")
361-
if err := os.MkdirAll(specsDir, 0755); err != nil {
361+
if err := os.MkdirAll(specsDir, 0o755); err != nil {
362362
t.Fatal(err)
363363
}
364364

@@ -370,7 +370,7 @@ Gem::Specification.new do |s|
370370
s.licenses = ["%s"]
371371
end
372372
`, name, version, license)
373-
if err := os.WriteFile(filepath.Join(specsDir, filename), []byte(content), 0644); err != nil {
373+
if err := os.WriteFile(filepath.Join(specsDir, filename), []byte(content), 0o600); err != nil {
374374
t.Fatal(err)
375375
}
376376
}
@@ -410,6 +410,7 @@ end
410410
}
411411

412412
func TestRubyGemfileLockResolver_PathTraversal(t *testing.T) {
413+
const evil = "evil"
413414
resolver := new(GemfileLockResolver)
414415
dir := t.TempDir()
415416

@@ -423,7 +424,7 @@ Gem::Specification.new do |s|
423424
s.version = "1.0.0"
424425
s.licenses = ["Evil-License"]
425426
end
426-
`), 0644); err != nil {
427+
`), 0o600); err != nil {
427428
t.Fatal(err)
428429
}
429430

@@ -466,7 +467,7 @@ BUNDLED WITH
466467

467468
found := false
468469
for _, r := range report.Resolved {
469-
if r.Dependency == "evil" {
470+
if r.Dependency == evil {
470471
found = true
471472
if r.LicenseSpdxID == "Evil-License" {
472473
t.Errorf("Path traversal succeeded! Found license from outside directory.")
@@ -475,7 +476,7 @@ BUNDLED WITH
475476
}
476477
// If it's skipped, that's also fine (means it didn't resolve license)
477478
for _, r := range report.Skipped {
478-
if r.Dependency == "evil" {
479+
if r.Dependency == evil {
479480
found = true
480481
}
481482
}

0 commit comments

Comments
 (0)