Merge pull request #23115 from Homebrew/vulns-osv-export-archaeology

generate-vulns-advisories: derive initial fixed via FormulaVersions

Library/Homebrew/dev-cmd/generate-vulns-advisories.rb

@@ -3,6 +3,7 @@
 
 require "abstract_command"
 require "formula"
+require "formula_versions"
 require "vulns/osv_export"
 
 module Homebrew
@@ -32,10 +33,10 @@ module Homebrew
         dir = args.named.first
 
         Formulary.enable_factory_cache!
-        annotated = Homebrew.with_no_api_env do
+        Homebrew.with_no_api_env do
           latest_macos = MacOSVersion.new((HOMEBREW_MACOS_NEWEST_UNSUPPORTED.to_i - 1).to_s).to_sym
           Homebrew::SimulateSystem.with(os: latest_macos, arch: :arm) do
-            tap.formula_names.filter_map do |name|
+            annotated = tap.formula_names.filter_map do |name|
               formula = Formulary.factory(name)
               patches = all_variation_patches(formula)
               [formula, patches] if Homebrew::Vulns::Scanner.resolved_ids(patches).any?
@@ -43,22 +44,25 @@ module Homebrew
               onoe "Error loading formula '#{name}'."
               raise
             end
-          end
-        end
-        ohai "#{annotated.size} formulae with security `resolves` annotations"
+            ohai "#{annotated.size} formulae with security `resolves` annotations"
 
-        if args.dry_run?
-          annotated.each do |formula, patches|
-            Homebrew::Vulns::Scanner.resolved_ids(patches).each do |vuln_id|
-              puts "#{Homebrew::Vulns::OsvExport::ID_PREFIX}-#{formula.name}-#{vuln_id}"
+            if args.dry_run?
+              annotated.each do |formula, patches|
+                Homebrew::Vulns::Scanner.resolved_ids(patches).each do |vuln_id|
+                  puts "#{Homebrew::Vulns::OsvExport::ID_PREFIX}-#{formula.name}-#{vuln_id}"
+                end
+              end
+              next
             end
-          end
-          return
-        end
 
-        written = Homebrew::Vulns::OsvExport.run(annotated, T.must(dir))
-        written.each { |p| puts "  wrote #{p}" } if args.verbose?
-        ohai "#{written.size} records written to #{dir}"
+            written = Homebrew::Vulns::OsvExport.run(
+              annotated, T.must(dir),
+              first_fixed: ->(formula, vuln_id) { first_fixed_version(formula, vuln_id) }
+            )
+            written.each { |p| puts "  wrote #{p}" } if args.verbose?
+            ohai "#{written.size} records written to #{dir}"
+          end
+        end
       end
 
       # `Formula#serialized_patches` reflects the currently simulated OS and
@@ -74,6 +78,52 @@ module Homebrew
         variation_patches = hash.fetch("variations").values.filter_map { |v| v["patches"] }
         (base + variation_patches.flatten(1)).uniq
       end
+
+      # Walk homebrew-core git history (newest first) via {FormulaVersions} and
+      # return the `pkg_version` at the oldest revision where `vuln_id` still
+      # appears in the formula's resolved patch ids: the version at which the
+      # fix first shipped. Revisions that fail to load (older DSL) end the walk
+      # early. Only invoked for records with no existing file, so the cost is
+      # bounded to newly annotated (formula, CVE) pairs.
+      #
+      # Because `resolved_ids` includes CVEs inferred from patch URLs and
+      # `apply` file paths, this finds the true fix version when the CVE is
+      # named there. When a `resolves` line was added to a patch that had
+      # already shipped without a CVE reference, it finds when `resolves` was
+      # added (too recent); those cases are hand-corrected in the advisory
+      # repository, which {Homebrew::Vulns::OsvExport.run} then preserves.
+      #
+      # Historical revisions are loaded under the enclosing {SimulateSystem}
+      # (latest macOS/ARM) only; a `resolves` that lives inside e.g. `on_linux`
+      # is invisible here and falls through to the current `pkg_version`.
+      # {FormulaVersions} caches by revision alone, so per-variation historical
+      # loading would need separate instances; deferred until a variation-only
+      # security annotation actually exists in core.
+      sig { params(formula: Formula, vuln_id: String).returns(T.nilable(String)) }
+      def first_fixed_version(formula, vuln_id)
+        # `FormulaVersions#rev_list` shells out to path-filtered `git rev-list`
+        # over the whole homebrew-core history and dominates runtime; cache it
+        # (and the instance, for its per-revision formula memoisation) per
+        # formula so subsequent CVEs for the same formula reuse both.
+        @formula_versions ||= T.let({}, T.nilable(T::Hash[String, FormulaVersions]))
+        @formula_rev_lists ||= T.let({}, T.nilable(T::Hash[String, T::Array[[String, String]]]))
+        fv = @formula_versions[formula.name] ||= FormulaVersions.new(formula)
+        revs = @formula_rev_lists[formula.name] ||=
+          [].tap { |a| fv.rev_list("HEAD") { |rev, entry| a << [rev, entry] } }
+
+        last_fixed = T.let(nil, T.nilable(String))
+        revs.each do |rev, entry|
+          resolved_here = fv.formula_at_revision(rev, entry) do |old|
+            Homebrew::Vulns::Scanner.resolved_ids(old.serialized_patches).include?(vuln_id)
+          end
+          # `nil` means the revision failed to load; stop rather than guess.
+          return last_fixed if resolved_here.nil?
+          return last_fixed unless resolved_here
+
+          last_fixed = fv.formula_at_revision(rev, entry) { |old| old.pkg_version.to_s }
+        end
+        last_fixed
+      end
     end
   end
 end

Library/Homebrew/test/dev-cmd/generate-vulns-advisories_spec.rb

@@ -30,6 +30,7 @@ RSpec.describe Homebrew::DevCmd::GenerateVulnsAdvisories do
     allow(Formulary).to receive(:factory).with("nvi").and_return(nvi)
     allow(Formulary).to receive(:factory).with("plain").and_return(plain)
     allow(Homebrew::Vulns::OSV).to receive(:vulnerability).and_return({})
+    allow(FormulaVersions).to receive(:new).and_return(instance_double(FormulaVersions, rev_list: nil))
 
     Dir.mktmpdir do |dir|
       out = "#{dir}/advisories"
@@ -97,4 +98,68 @@ RSpec.describe Homebrew::DevCmd::GenerateVulnsAdvisories do
       expect(cmd.all_variation_patches(f)).to eq [{ "url" => "a" }]
     end
   end
+
+  describe "#first_fixed_version" do
+    subject(:cmd) { described_class.new(["out"]) }
+
+    let(:current) { formula("x") { url "https://example.com/x-1.2.tar.gz" } }
+
+    def with_history(revisions)
+      fv = instance_double(FormulaVersions)
+      allow(FormulaVersions).to receive(:new).with(current).and_return(fv)
+      allow(fv).to receive(:rev_list).with("HEAD") do |&blk|
+        revisions.each_key { |rev| blk.call(rev, "Formula/x/x.rb") }
+      end
+      allow(fv).to receive(:formula_at_revision) do |rev, _entry, &blk|
+        old = revisions.fetch(rev)
+        old.nil? ? nil : blk.call(old)
+      end
+    end
+
+    def old_formula(pkg_version:, resolves_ids: [])
+      patches = resolves_ids.map { |id| { "resolves" => [{ "type" => "security", "id" => id }] } }
+      instance_double(Formula, pkg_version: PkgVersion.parse(pkg_version), serialized_patches: patches)
+    end
+
+    it "returns the pkg_version at the oldest revision where the CVE is resolved" do
+      with_history(
+        "r3" => old_formula(pkg_version: "1.2_1", resolves_ids: ["CVE-2024-1"]),
+        "r2" => old_formula(pkg_version: "1.2", resolves_ids: ["CVE-2024-1"]),
+        "r1" => old_formula(pkg_version: "1.1", resolves_ids: []),
+        # Trap: if the walk continued past r1 it would wrongly return 1.0.
+        "r0" => old_formula(pkg_version: "1.0", resolves_ids: ["CVE-2024-1"]),
+      )
+
+      expect(cmd.first_fixed_version(current, "CVE-2024-1")).to eq "1.2"
+    end
+
+    it "returns the oldest resolved version when the CVE is resolved in every revision" do
+      with_history(
+        "r2" => old_formula(pkg_version: "1.1", resolves_ids: ["CVE-2024-1"]),
+        "r1" => old_formula(pkg_version: "1.0", resolves_ids: ["CVE-2024-1"]),
+      )
+
+      expect(cmd.first_fixed_version(current, "CVE-2024-1")).to eq "1.0"
+    end
+
+    it "stops at an unloadable revision and returns the last known resolved version" do
+      with_history(
+        "r3" => old_formula(pkg_version: "1.2", resolves_ids: ["CVE-2024-1"]),
+        "r2" => nil,
+        "r1" => old_formula(pkg_version: "1.0", resolves_ids: ["CVE-2024-1"]),
+      )
+
+      expect(cmd.first_fixed_version(current, "CVE-2024-1")).to eq "1.2"
+    end
+
+    it "returns nil when the CVE is not resolved at the newest revision" do
+      with_history(
+        "r2" => old_formula(pkg_version: "1.2", resolves_ids: []),
+        # Trap: if the walk continued past r2 it would wrongly return 1.0.
+        "r1" => old_formula(pkg_version: "1.0", resolves_ids: ["CVE-2024-1"]),
+      )
+
+      expect(cmd.first_fixed_version(current, "CVE-2024-1")).to be_nil
+    end
+  end
 end

Library/Homebrew/test/vulns/osv_export_spec.rb

@@ -154,6 +154,13 @@ RSpec.describe Homebrew::Vulns::OsvExport do
       expect(affected[:package][:name]).to eq "libsigc++"
     end
 
+    it "uses an explicit fixed boundary over the current pkg_version" do
+      events = described_class.record_for(nvi, "CVE-2015-2305", fixed: "1.81.6_3", now:)
+                              .dig(:affected, 0, :ranges, 0, :events)
+
+      expect(events[1]).to eq({ fixed: "1.81.6_3" })
+    end
+
     it "omits the revision suffix when revision is zero" do
       f = formula("x") do
         url "https://example.com/x-1.0.tar.gz"
@@ -190,6 +197,37 @@ RSpec.describe Homebrew::Vulns::OsvExport do
       end
     end
 
+    it "calls first_fixed only for records with no existing file" do
+      allow(Homebrew::Vulns::OSV).to receive(:vulnerability).and_return({})
+      calls = []
+      first_fixed = lambda { |f, id|
+        calls << [f.name, id]
+        "1.2.4_2"
+      }
+
+      Dir.mktmpdir do |dir|
+        File.write("#{dir}/BREW-libquicktime-CVE-2016-2399.json",
+                   JSON.generate(described_class.record_for(libquicktime, "CVE-2016-2399", now:)))
+
+        described_class.run([[libquicktime, libquicktime.serialized_patches]], dir, first_fixed:, now:)
+
+        expect(calls).to eq [["libquicktime", "CVE-2017-9122"]]
+        record = JSON.parse(File.read("#{dir}/BREW-libquicktime-CVE-2017-9122.json"))
+        expect(record["affected"][0]["ranges"][0]["events"][1]).to eq({ "fixed" => "1.2.4_2" })
+      end
+    end
+
+    it "falls back to pkg_version when first_fixed returns nil" do
+      allow(Homebrew::Vulns::OSV).to receive(:vulnerability).and_return({})
+
+      Dir.mktmpdir do |dir|
+        described_class.run([[nvi, nvi.serialized_patches]], dir, first_fixed: ->(_f, _id) {}, now:)
+
+        record = JSON.parse(File.read("#{dir}/BREW-nvi-CVE-2015-2305.json"))
+        expect(record["affected"][0]["ranges"][0]["events"][1]).to eq({ "fixed" => "1.81.6_6" })
+      end
+    end
+
     it "fetches each upstream vuln id once, even when shared across formulae" do
       shared = [{ "url"      => "https://example.com/fix.patch",
                   "resolves" => [{ "type" => "security", "id" => "CVE-2024-9999" }] }]

Library/Homebrew/vulns/osv_export.rb

@@ -38,12 +38,19 @@ module Homebrew
       # supply the union across OS/architecture variations (a `patch` inside an
       # `on_linux`/`on_intel` block only appears in `Formula#serialized_patches`
       # under the matching {SimulateSystem}).
+      #
+      # `first_fixed`, when given, is called `(formula, vuln_id) -> String?` for
+      # records with no existing file to derive an accurate `fixed` boundary
+      # (e.g. via {FormulaVersions} git history); existing records preserve
+      # their on-disk `ranges` regardless.
       sig {
-        params(annotated: T::Array[[Formula, T::Array[T::Hash[String, T.untyped]]]],
-               dir:       T.any(String, Pathname), now: Time)
+        params(annotated:   T::Array[[Formula, T::Array[T::Hash[String, T.untyped]]]],
+               dir:         T.any(String, Pathname),
+               first_fixed: T.nilable(T.proc.params(formula: Formula, vuln_id: String).returns(T.nilable(String))),
+               now:         Time)
           .returns(T::Array[String])
       }
-      def self.run(annotated, dir, now: Time.now.utc)
+      def self.run(annotated, dir, first_fixed: nil, now: Time.now.utc)
         FileUtils.mkdir_p(dir)
         written = []
         upstream_cache = T.let({}, T::Hash[String, T.any(T::Hash[String, T.untyped], Symbol)])
@@ -52,11 +59,13 @@ module Homebrew
           Scanner.resolved_ids(patches).each do |vuln_id|
             upstream = upstream_cache.fetch(vuln_id) { upstream_cache[vuln_id] = fetch_upstream(vuln_id) }
             path = File.join(dir, "#{ID_PREFIX}-#{formula.name}-#{vuln_id}.json")
+            existing = File.file?(path)
             # A transient OSV outage would otherwise strip summary/severity/etc.
             # from an existing enriched record; leave it untouched instead.
-            next if upstream == :failed && File.file?(path)
+            next if upstream == :failed && existing
 
-            record = record_for(formula, vuln_id, patches:,
+            fixed = (first_fixed&.call(formula, vuln_id) unless existing) || formula.pkg_version.to_s
+            record = record_for(formula, vuln_id, patches:, fixed:,
                                 upstream: upstream.is_a?(Hash) ? upstream : nil, now:)
             merged = merge_existing(path, record)
             next if merged.nil?
@@ -105,10 +114,11 @@ module Homebrew
 
       sig {
         params(formula: Formula, vuln_id: String, patches: T::Array[T::Hash[String, T.untyped]],
-               upstream: T.nilable(T::Hash[String, T.untyped]), now: Time)
+               fixed: String, upstream: T.nilable(T::Hash[String, T.untyped]), now: Time)
           .returns(T::Hash[Symbol, T.untyped])
       }
-      def self.record_for(formula, vuln_id, patches: formula.serialized_patches, upstream: nil, now: Time.now.utc)
+      def self.record_for(formula, vuln_id, patches: formula.serialized_patches,
+                          fixed: formula.pkg_version.to_s, upstream: nil, now: Time.now.utc)
         timestamp = now.strftime("%Y-%m-%dT%H:%M:%SZ")
         record = T.let({
           schema_version:    SCHEMA_VERSION,
@@ -116,7 +126,7 @@ module Homebrew
           published:         timestamp,
           modified:          timestamp,
           upstream:          [vuln_id],
-          affected:          [affected_entry(formula, vuln_id, patches)],
+          affected:          [affected_entry(formula, vuln_id, patches, fixed)],
           database_specific: { source: "generated" },
         }, T::Hash[Symbol, T.untyped])
 
@@ -132,10 +142,10 @@ module Homebrew
       end
 
       sig {
-        params(formula: Formula, vuln_id: String, patches: T::Array[T::Hash[String, T.untyped]])
+        params(formula: Formula, vuln_id: String, patches: T::Array[T::Hash[String, T.untyped]], fixed: String)
           .returns(T::Hash[Symbol, T.untyped])
       }
-      def self.affected_entry(formula, vuln_id, patches)
+      def self.affected_entry(formula, vuln_id, patches, fixed)
         {
           package:            {
             ecosystem: ECOSYSTEM,
@@ -145,7 +155,7 @@ module Homebrew
           ranges:             [
             {
               type:   "ECOSYSTEM",
-              events: [{ introduced: "0" }, { fixed: formula.pkg_version.to_s }],
+              events: [{ introduced: "0" }, { fixed: }],
             },
           ],
           ecosystem_specific: {