Summarize changes in regeneration pull requests

.github/workflows/regenerate.yml

@@ -74,6 +74,7 @@ jobs:
             echo "No changes."
             exit 0
           fi
+          bundle exec rake advisories:summary > "${RUNNER_TEMP}/regenerate-pr-body.md"
           git checkout -b "${BRANCH}"
           git commit -m "Regenerate advisories"
           {
@@ -92,19 +93,20 @@ jobs:
           no_lease: true
           origin_branch: HEAD
 
-      - name: Open pull request
+      - name: Open or update pull request
         if: steps.commit.outputs.committed == 'true'
         env:
           GITHUB_TOKEN: ${{ secrets.HOMEBREW_GITHUB_PUBLIC_REPO_TOKEN || github.token }}
           BRANCH: ${{ steps.commit.outputs.branch }}
+          BODY: ${{ runner.temp }}/regenerate-pr-body.md
         run: |
           open_pr_number="$(gh pr list --head "${BRANCH}" --state open \
             --json number,isCrossRepository \
             --jq '.[] | select(.isCrossRepository | not) | .number')"
           if [[ -n "${open_pr_number}" ]]; then
-            echo "PR already open."
+            gh pr edit "${open_pr_number}" --body-file "${BODY}"
           else
             gh pr create --base main --head "${BRANCH}" \
               --title "Regenerate advisories" \
-              --body "Automated regeneration via \`brew generate-vulns-advisories\`."
+              --body-file "${BODY}"
           fi

Rakefile

@@ -35,6 +35,20 @@ namespace :advisories do
     AdvisoryIndex.write("advisories", "data/advisories.json")
   end
 
+  desc "Summarize data/advisories.json changes against HEAD as Markdown"
+  task :summary do
+    require "json"
+    require "open3"
+    require_relative "lib/advisory_change_summary"
+
+    before_json, status = Open3.capture2("git", "show", "HEAD:data/advisories.json")
+    abort "could not read data/advisories.json from HEAD" unless status.success?
+
+    before = JSON.parse(before_json)
+    after = JSON.parse(File.read("data/advisories.json"))
+    puts AdvisoryChangeSummary.render(before, after)
+  end
+
   desc "Delete uncomparable and rejected matched candidates named as untracked paths on standard input"
   task :filter do
     require_relative "lib/advisory_filter"

lib/advisory_change_summary.rb

@@ -0,0 +1,115 @@
+# frozen_string_literal: true
+
+# Renders a concise pull request summary for changes to data/advisories.json.
+module AdvisoryChangeSummary
+  module_function
+
+  def render(before, after, formula_limit: 50)
+    before_records = records_by_id(before)
+    after_records = records_by_id(after)
+    added = (after_records.keys - before_records.keys).to_h { |id| [id, after_records.fetch(id)] }
+    removed = (before_records.keys - after_records.keys).to_h { |id| [id, before_records.fetch(id)] }
+    updated = (before_records.keys & after_records.keys)
+              .reject { |id| before_records.fetch(id) == after_records.fetch(id) }
+              .to_h { |id| [id, after_records.fetch(id)] }
+
+    lines = [
+      "Automated regeneration via `brew generate-vulns-advisories`, followed by rebuilding " \
+      "`data/advisories.json`.",
+      "",
+    ]
+    lines << change_line("Added", added.values) unless added.empty?
+    lines << change_line("Updated", updated.values) unless updated.empty?
+    lines << change_line("Removed", removed.values) unless removed.empty?
+    lines << "- No indexed advisory records changed." if added.empty? && updated.empty? && removed.empty?
+
+    formulae = changed_formulae(before_records, after_records, added.keys, updated.keys, removed.keys)
+    after_meta = after.fetch("meta")
+    total = after_meta.fetch("count")
+    formula_count = after.fetch("advisories").size
+    lines << "- The index now contains #{format_number(total)} #{pluralize(total, "advisory", "advisories")} " \
+             "across #{format_number(formula_count)} #{pluralize(formula_count, "formula", "formulae")}."
+
+    before_meta = before.fetch("meta")
+    before_skipped = before_meta.fetch("skipped_uncomparable", 0)
+    after_skipped = after_meta.fetch("skipped_uncomparable", 0)
+    if before_skipped != after_skipped
+      lines << "- Skipped uncomparable advisories: #{format_number(after_skipped)} " \
+               "(previously #{format_number(before_skipped)})."
+    end
+
+    before_schema = before_meta["schema_version"]
+    after_schema = after_meta["schema_version"]
+    if before_schema != after_schema
+      lines << "- Schema version: `#{after_schema}` (previously `#{before_schema}`)."
+    end
+
+    unless formulae.empty?
+      lines << ""
+      lines.concat(formula_lines(formulae, formula_limit:))
+    end
+
+    lines.join("\n")
+  end
+
+  def records_by_id(index)
+    index.fetch("advisories").values.flatten.each_with_object({}) do |record, records|
+      id = record.fetch("id")
+      raise "duplicate advisory id: #{id}" if records.key?(id)
+
+      records[id] = record
+    end
+  end
+
+  def changed_formulae(before_records, after_records, added_ids, updated_ids, removed_ids)
+    records = added_ids.map { |id| after_records.fetch(id) }
+    records.concat(removed_ids.map { |id| before_records.fetch(id) })
+    updated_ids.each do |id|
+      records << before_records.fetch(id)
+      records << after_records.fetch(id)
+    end
+    records.flat_map { |record| formulae_for(record) }.uniq.sort
+  end
+
+  def formulae_for(record)
+    Array(record["affected"]).filter_map { |affected| affected.dig("package", "name") }
+  end
+
+  def change_line(verb, records)
+    groups = records.group_by { |record| source_for(record) }
+                    .transform_values(&:size)
+                    .sort
+    count = records.size
+    noun = pluralize(count, "advisory", "advisories")
+    return "- #{verb} #{format_number(count)} #{groups.first.first} #{noun}." if groups.one?
+
+    details = groups.map { |source, source_count| "#{format_number(source_count)} #{source}" }.join(", ")
+    "- #{verb} #{format_number(count)} #{noun}: #{details}."
+  end
+
+  def source_for(record)
+    record.dig("database_specific", "source") || "manually maintained"
+  end
+
+  def formula_lines(formulae, formula_limit:)
+    visible = formulae.first(formula_limit)
+    remaining = formulae.size - visible.size
+    lines = [
+      "Affected #{pluralize(formulae.size, "formula", "formulae")} (#{format_number(formulae.size)}):",
+      "",
+      *visible.map { |formula| "- `#{formula}`" },
+    ]
+    if remaining.positive?
+      lines << "- #{format_number(remaining)} more #{pluralize(remaining, "formula", "formulae")} omitted."
+    end
+    lines
+  end
+
+  def pluralize(count, singular, plural)
+    (count == 1) ? singular : plural
+  end
+
+  def format_number(number)
+    number.to_s.reverse.scan(/\d{1,3}/).join(",").reverse
+  end
+end

spec/advisory_change_summary_spec.rb

@@ -0,0 +1,103 @@
+# frozen_string_literal: true
+
+require "advisory_change_summary"
+
+RSpec.describe AdvisoryChangeSummary do
+  def record(id, formula, source: nil, modified: "old")
+    advisory = {
+      "id"       => id,
+      "modified" => modified,
+      "affected" => [{ "package" => { "name" => formula } }],
+    }
+    advisory["database_specific"] = { "source" => source } if source
+    advisory
+  end
+
+  def index(*records, skipped: 0, schema: "1.7.3")
+    {
+      "meta"       => {
+        "count"                => records.size,
+        "skipped_uncomparable" => skipped,
+        "schema_version"       => schema,
+      },
+      "advisories" => records.group_by { |advisory| advisory.dig("affected", 0, "package", "name") },
+    }
+  end
+
+  it "summarizes added, updated, and removed records by source" do
+    before = index(
+      record("BREW-foo-CVE-1", "foo", source: "generated"),
+      record("BREW-bar-CVE-1", "bar"),
+    )
+    after = index(
+      record("BREW-foo-CVE-1", "foo", source: "generated", modified: "new"),
+      record("BREW-baz-CVE-1", "baz", source: "matched"),
+    )
+
+    expect(described_class.render(before, after)).to eq <<~MARKDOWN.chomp
+      Automated regeneration via `brew generate-vulns-advisories`, followed by rebuilding `data/advisories.json`.
+
+      - Added 1 matched advisory.
+      - Updated 1 generated advisory.
+      - Removed 1 manually maintained advisory.
+      - The index now contains 2 advisories across 2 formulae.
+
+      Affected formulae (3):
+
+      - `bar`
+      - `baz`
+      - `foo`
+    MARKDOWN
+  end
+
+  it "caps the formula list" do
+    after = index(
+      record("BREW-a-CVE-1", "a", source: "generated"),
+      record("BREW-b-CVE-1", "b", source: "generated"),
+      record("BREW-c-CVE-1", "c", source: "generated"),
+    )
+
+    summary = described_class.render(index, after, formula_limit: 2)
+
+    expect(summary).to include("Affected formulae (3):\n\n- `a`\n- `b`\n- 1 more formula omitted.")
+    expect(summary).not_to include("- `c`")
+  end
+
+  it "formats large counts for readability" do
+    records = Array.new(1_001) do |number|
+      record("BREW-foo-CVE-#{number}", "foo", source: "matched")
+    end
+
+    summary = described_class.render(index, index(*records))
+
+    expect(summary).to include("- Added 1,001 matched advisories.")
+    expect(summary).to include("- The index now contains 1,001 advisories across 1 formula.")
+  end
+
+  it "reports metadata changes without record changes" do
+    before = index(skipped: 1, schema: "1.7.2")
+    after = index(skipped: 3, schema: "1.7.3")
+
+    summary = described_class.render(before, after)
+
+    expect(summary).to include("- No indexed advisory records changed.")
+    expect(summary).to include("- Skipped uncomparable advisories: 3 (previously 1).")
+    expect(summary).to include("- Schema version: `1.7.3` (previously `1.7.2`).")
+  end
+
+  it "includes both formulae when an updated advisory moves" do
+    before = index(record("BREW-formula-CVE-1", "old-name", source: "generated"))
+    after = index(record("BREW-formula-CVE-1", "new-name", source: "generated"))
+
+    expect(described_class.render(before, after))
+      .to include("Affected formulae (2):\n\n- `new-name`\n- `old-name`")
+  end
+
+  it "rejects duplicate advisory ids" do
+    duplicate = record("BREW-foo-CVE-1", "foo", source: "generated")
+    after = index(duplicate, duplicate)
+
+    expect { described_class.render(index, after) }
+      .to raise_error("duplicate advisory id: BREW-foo-CVE-1")
+  end
+end

spec/workflows_spec.rb

@@ -34,11 +34,26 @@
                                      aliases: true)
     steps = regenerate.dig("jobs", "regenerate", "steps")
     push = steps.find { |step| step["name"] == "Push commits" }
-    open_pr = steps.find { |step| step["name"] == "Open pull request" }
+    open_pr = steps.find { |step| step["name"] == "Open or update pull request" }
     expect(push.fetch("with")).to include("token" => machine_token)
     expect(open_pr.dig("env", "GITHUB_TOKEN")).to eq machine_token
   end
 
+  it "summarizes regeneration changes and refreshes the standing pull request body" do
+    path = File.expand_path("../.github/workflows/regenerate.yml", __dir__)
+    workflow = YAML.safe_load_file(path, aliases: true)
+    steps = workflow.dig("jobs", "regenerate", "steps")
+    commit = steps.find { |step| step["name"] == "Commit advisories" }.fetch("run")
+    open_pr = steps.find { |step| step["name"] == "Open or update pull request" }
+    publish = open_pr.fetch("run")
+
+    expect(commit).to include("bundle exec rake advisories:summary")
+    expect(open_pr.dig("env", "BODY")).to eq "${{ runner.temp }}/regenerate-pr-body.md"
+    expect(publish).to include('gh pr edit "${open_pr_number}" --body-file "${BODY}"')
+    expect(publish).to include('--body-file "${BODY}"')
+    expect(publish).not_to include("--body ")
+  end
+
   it "builds the Repology index in Ingest and reads it via --repology" do
     regenerate = YAML.safe_load_file(File.expand_path("../.github/workflows/regenerate.yml", __dir__),
                                      aliases: true)