Merge pull request #629 from github/reviewed-and-ignored-at-versions

Add support for configuring reviewed and ignored dependencies at specific versions

docs/configuration/ignoring_dependencies.md

@@ -17,3 +17,16 @@ ignored:
   go:
     - github.com/me/my-repo/**/*
 ```
+
+## Ignoring dependencies at specific versions
+
+Ignore a dependency at specific versions by appending `@<version>` to the end of the dependency's name in an `ignore` list.  If a dependency is configured to be ignored at a specific version, licensed will not ignore non-matching versions of the dependency.
+
+The version value can be one of:
+
+1. `"*"` - match any version value
+1. any version string, or version range string, that can be parsed by `Gem::Requirement`
+   - a semantic version - `dependency@1.2.3`
+   - a gem requirement range - `dependency@~> 1.0.0` or `dependency@< 3.0`
+   - see the [Rubygems version guides](https://guides.rubygems.org/patterns/#pessimistic-version-constraint) for more details about specifying gem version requirements
+1. a value that can't be parsed by `Gem::Requirement`, which will only match dependencies with the same version string

docs/configuration/reviewing_dependencies.md

@@ -16,3 +16,16 @@ reviewed:
   bundler:
     - gem-using-unallowed-license
 ```
+
+## Reviewing dependencies at specific versions
+
+Review a dependency at specific versions by appending `@<version>` to the end of the dependency's name in an `reviewed` list.  If a dependency is configured to be reviewed at a specific version, licensed will not recognize non-matching versions of the dependency as being manually reviewed and accepted.
+
+The version value can be one of:
+
+1. `"*"` - match any version value
+1. any version string, or version range string, that can be parsed by `Gem::Requirement`
+   - a semantic version - `dependency@1.2.3`
+   - a gem requirement range - `dependency@~> 1.0.0` or `dependency@< 3.0`
+   - see the [Rubygems version guides](https://guides.rubygems.org/patterns/#pessimistic-version-constraint) for more details about specifying gem version requirements
+1. a value that can't be parsed by `Gem::Requirement`, which will only match dependencies with the same version string

docs/sources/pnpm.md

@@ -2,6 +2,8 @@
 
 The npm source will detect dependencies when `pnpm-lock.yaml` is found at an apps `source_path`.  It uses `pnpm licenses list` to enumerate dependencies and metadata.
 
+All dependencies enumerated by the pnpm source include the dependency version in the dependency's name identifier.  All [reviewed](../configuration/reviewing_dependencies.md) or [ignored](../configuration/ignoring_dependencies.md) dependencies must include a version signifier in the configured dependency name.
+
 **NOTE** [pnpm licenses list](https://pnpm.io/cli/licenses) is an experimental CLI command and subject to change.  If changes to pnpm result in unexpected or broken behavior in licensed please open an [issue](https://github.com/github/licensed/issues/new).
 
 ## Including development dependencies

lib/licensed/commands/cache.rb

@@ -83,7 +83,7 @@ def evaluate_dependency(app, source, dependency, report)
         report["version"] = dependency.version
 
         if save_dependency_record?(dependency, cached_record)
-          update_dependency_from_cached_record(app, dependency, cached_record)
+          update_dependency_from_cached_record(app, source, dependency, cached_record)
 
           dependency.record.save(filename)
           report["cached"] = true
@@ -123,14 +123,14 @@ def save_dependency_record?(dependency, cached_record)
       # Update dependency metadata from the cached record, to support:
       # 1. continuity between cache runs to cut down on churn
       # 2. notifying users when changed content needs to be reviewed
-      def update_dependency_from_cached_record(app, dependency, cached_record)
+      def update_dependency_from_cached_record(app, source, dependency, cached_record)
         return if cached_record.nil?
         return if options[:force]
 
         if dependency.record.matches?(cached_record)
           # use the cached license value if the license text wasn't updated
           dependency.record["license"] = cached_record["license"]
-        elsif app.reviewed?(dependency.record)
+        elsif app.reviewed?(dependency.record, require_version: source.class.require_matched_dependency_version)
           # if the license text changed and the dependency is set as reviewed
           # force a re-review of the dependency
           dependency.record["review_changed_license"] = true

lib/licensed/commands/status.rb

@@ -60,7 +60,7 @@ def evaluate_dependency(app, source, dependency, report)
           report.errors << "missing license text" if record.licenses.empty?
           if record["review_changed_license"]
             report.errors << "license text has changed and needs re-review. if the new text is ok, remove the `review_changed_license` flag from the cached record"
-          elsif license_needs_review?(app, record)
+          elsif license_needs_review?(app, source, record)
             report.errors << needs_review_error_message(app, record)
           end
         end
@@ -70,9 +70,10 @@ def evaluate_dependency(app, source, dependency, report)
 
       # Returns true if a cached record needs further review based on the
       # record's license(s) and the app's configuration
-      def license_needs_review?(app, record)
+      def license_needs_review?(app, source, record)
         # review is not needed if the record is set as reviewed
-        return false if app.reviewed?(record, match_version: data_source == "configuration")
+        require_version = data_source == "configuration" || source.class.require_matched_dependency_version
+        return false if app.reviewed?(record, require_version: require_version)
 
         # review is not needed if the top level license is allowed
         return false if app.allowed?(record["license"])
@@ -99,7 +100,7 @@ def needs_review_error_message(app, record)
         error = "dependency needs review"
 
         # look for an unversioned reviewed list match
-        if app.reviewed?(record, match_version: false)
+        if app.reviewed?(record, require_version: false)
           error += ", unversioned 'reviewed' match found: #{record["name"]}"
         end
 

lib/licensed/configuration.rb

@@ -10,6 +10,8 @@ class AppConfiguration < Hash
 
     DEFAULT_CACHE_PATH = ".licenses".freeze
 
+    ANY_VERSION_REQUIREMENT = "*".freeze
+
     # Returns the root for a configuration in following order of precedence:
     # 1. explicitly configured "root" property
     # 2. a found git repository root
@@ -73,8 +75,8 @@ def enabled?(source_type)
     end
 
     # Is the given dependency reviewed?
-    def reviewed?(dependency, match_version: false)
-      any_list_pattern_matched? self["reviewed"][dependency["type"]], dependency, match_version: match_version
+    def reviewed?(dependency, require_version: false)
+      any_list_pattern_matched? self["reviewed"][dependency["type"]], dependency, require_version: require_version
     end
 
     # Find all reviewed dependencies that match the provided dependency's name
@@ -83,8 +85,8 @@ def reviewed_versions(dependency)
     end
 
     # Is the given dependency ignored?
-    def ignored?(dependency)
-      any_list_pattern_matched? self["ignored"][dependency["type"]], dependency
+    def ignored?(dependency, require_version: false)
+      any_list_pattern_matched? self["ignored"][dependency["type"]], dependency, require_version: require_version
     end
 
     # Is the license of the dependency allowed?
@@ -93,8 +95,10 @@ def allowed?(license)
     end
 
     # Ignore a dependency
-    def ignore(dependency)
-      (self["ignored"][dependency["type"]] ||= []) << dependency["name"]
+    def ignore(dependency, at_version: false)
+      id = dependency["name"]
+      id += "@#{dependency["version"]}" if at_version && dependency["version"]
+      (self["ignored"][dependency["type"]] ||= []) << id
     end
 
     # Set a dependency as reviewed
@@ -117,15 +121,41 @@ def additional_terms_for_dependency(dependency)
 
     private
 
-    def any_list_pattern_matched?(list, dependency, match_version: false)
+    def any_list_pattern_matched?(list, dependency, require_version: false)
       Array(list).any? do |pattern|
-        if match_version
-          at_version = "@#{dependency["version"]}"
-          pattern, pattern_version = pattern.rpartition(at_version).values_at(0, 1)
-          next false if pattern == "" || pattern_version == ""
+        # parse a name and version requirement value from the pattern
+        name, requirement = pattern.rpartition("@").values_at(0, 2).map(&:strip)
+
+        if name == ""
+          # if name == "", then the pattern doesn't contain a valid version value.
+          # treat the entire pattern as the dependency name with no version.
+          name = pattern
+          requirement = nil
+        elsif !requirement.to_s.empty?
+          # check if the version requirement is a valid Gem::Requirement
+          # for range matching
+          requirements = requirement.split(",").map(&:strip)
+          if requirements.all? { |r| Gem::Requirement::PATTERN.match?(r) }
+            requirement = Gem::Requirement.new(requirements)
+          end
         end
 
-        File.fnmatch?(pattern, dependency["name"], File::FNM_PATHNAME | File::FNM_CASEFOLD)
+        # the pattern's name must match the dependency's name
+        next false unless File.fnmatch?(name, dependency["name"], File::FNM_PATHNAME | File::FNM_CASEFOLD)
+
+        # if there is no version requirement configured or if the dependency doesn't have a version
+        # specified, return a value based on whether a version match is required
+        next !require_version if requirement.nil? || dependency["version"].to_s.empty?
+
+        case requirement
+        when String
+          # string match the requirement against "*" or the dependency's version
+          [ANY_VERSION_REQUIREMENT, dependency["version"]].any? { |r| requirement == r }
+        when Gem::Requirement
+          # if the version was parsed as a gem requirement, check whether the version requirement
+          # matches the dependency's version
+          Gem::Version.correct?(dependency["version"]) && requirement.satisfied_by?(Gem::Version.new(dependency["version"]))
+        end
       end
     end
 

lib/licensed/dependency.rb

@@ -99,6 +99,17 @@ def notice_contents
          .select { |notice| notice["text"].length > 0 } # files with content only
     end
 
+    # Returns a hash of basic metadata about the dependency - name, version, type, etc
+    def metadata
+      {
+        # can be overriden by values in @metadata
+        "name" => name,
+        "version" => version
+      }.merge(
+        @metadata
+      )
+    end
+
     private
 
     def read_file_with_encoding_check(file_path)
@@ -135,14 +146,8 @@ def normalize_source_path(path)
     # Returns the metadata that represents this dependency.  This metadata
     # is written to YAML in the dependencys cached text file
     def license_metadata
-      {
-        # can be overriden by values in @metadata
-        "name" => name,
-        "version" => version
-      }.merge(
-        @metadata
-      ).merge({
-        # overrides all other values
+      metadata.merge({
+        # overrides all metadata values
         "license" => license_key
       })
     end

lib/licensed/sources/pnpm.rb

@@ -4,6 +4,12 @@
 module Licensed
   module Sources
     class PNPM < Source
+      # The PNPM source requires matching reviewed or ignored dependencies
+      # on both name and version
+      def self.require_matched_dependency_version
+        true
+      end
+
       # Returns true when pnpm is installed and a pnpm-lock.yaml file is found,
       # otherwise false
       def enabled?

lib/licensed/sources/source.rb

@@ -51,6 +51,12 @@ def type_and_version
                    .downcase
                    .split("::")
         end
+
+        # Returns true if the source requires matching reviewed and ignored dependencies'
+        # versions as well as their name
+        def require_matched_dependency_version
+          false
+        end
       end
 
       # all sources have a configuration
@@ -81,7 +87,7 @@ def enumerate_dependencies
 
       # Returns whether a dependency is ignored in the configuration.
       def ignored?(dependency)
-        config.ignored?("type" => self.class.type, "name" => dependency.name)
+        config.ignored?(dependency.metadata, require_version: self.class.require_matched_dependency_version)
       end
 
       private

test/commands/cache_test.rb

@@ -61,7 +61,7 @@
     config.apps.each do |app|
       FileUtils.mkdir_p app.cache_path.join("test")
       File.write app.cache_path.join("test/dependency.#{Licensed::DependencyRecord::EXTENSION}"), ""
-      app.ignore "type" => "test", "name" => "dependency"
+      app.ignore({ "type" => "test", "name" => "dependency" })
     end
 
     run_command
@@ -206,7 +206,7 @@
     config.apps.each do |app|
       FileUtils.mkdir_p app.cache_path.join("test")
       File.write app.cache_path.join("test/dependency.#{Licensed::DependencyRecord::EXTENSION}"), ""
-      app.ignore "type" => "test", "name" => "dependency"
+      app.ignore({ "type" => "test", "name" => "dependency" })
     end
 
     run_command

test/commands/list_test.rb

@@ -44,7 +44,7 @@
     count = dependencies.size
 
     config.apps.each do |app|
-      app.ignore("type" => "test", "name" => "dependency")
+      app.ignore({ "type" => "test", "name" => "dependency" })
     end
 
     run_command