Skip to content

vulns/osv_export: merge into existing records instead of overwriting#23112

Merged
andrew merged 3 commits into
mainfrom
vulns-osv-export-merge
Jul 14, 2026
Merged

vulns/osv_export: merge into existing records instead of overwriting#23112
andrew merged 3 commits into
mainfrom
vulns-osv-export-merge

Conversation

@andrew

@andrew andrew commented Jul 14, 2026

Copy link
Copy Markdown
Member

Follow-up to #23106 so Homebrew/homebrew-advisory-database can be an accumulating source of truth rather than a daily snapshot (per the outcome of Homebrew/homebrew-brew-vulns#111).

OsvExport.run now merges into the target directory instead of blindly writing. For each generated record, if an <id>.json already exists its published timestamp and affected[].ranges are carried forward, so the fixed boundary reflects when the annotation was first observed rather than drifting to today's pkg_version on every run; the write is skipped entirely when nothing else has changed. Records for annotations no longer in homebrew/core are simply not visited, so they persist on the default branch. Curated records (which use a different id shape) are never touched.

Because ranges is preserved from disk, a hand-corrected fixed boundary (e.g. backdating libquicktime to _4) survives subsequent runs. A transient OSV.dev failure leaves an existing enriched record untouched rather than stripping its summary/severity/references; a new record with no existing file is still written without enrichment. The unchanged comparison uses parsed hashes so key ordering in a hand-formatted file does not cause spurious rewrites.

Records also gain published and database_specific: {source: "generated"} per the draft CONTRIBUTING.md in Homebrew/advisory-database#3. regenerate.yml in that repo drops its rm -f advisories/*.json in Homebrew/advisory-database#2.


  • Have you followed our Contributing guidelines?
  • Have you checked for other open Pull Requests for the same change?
  • Have you explained what your changes do? Performance claims (e.g. "this is faster") must include Hyperfine benchmarks.
  • Have you explained why you'd like these changes included, not just what they do?
  • For bug fixes, have you given step-by-step brew commands to reproduce the bug?
  • Have you written new tests (excluding integration tests)? Here's an example.
  • Have you successfully run brew lgtm (style, typechecking and tests) locally?

  • AI was used to generate or assist with generating this PR.

Written with Claude Code following the discussion on Homebrew/homebrew-brew-vulns#111 and Homebrew/discussions#6869, with a codex review pass that caught the transient-outage stripping and key-order comparison issues. I read the diff and ran brew lgtm locally.


Records now carry `published` and `database_specific: {source:
"generated"}`. `.run` reads any existing `<id>.json`, preserves its
`published` timestamp and `affected[].ranges` (so the `fixed` boundary
reflects when the annotation was first observed rather than drifting to
the current pkg_version), and only writes when the remaining content has
changed. Records for annotations no longer in core are simply not
visited, so they persist on the default branch.

A transient OSV.dev failure leaves an existing enriched record untouched
rather than stripping its summary/severity/references. The unchanged
comparison uses parsed hashes so key ordering in a hand-formatted
existing file does not cause spurious rewrites.
Copilot AI review requested due to automatic review settings July 14, 2026 19:24
andrew added a commit to Homebrew/advisory-database that referenced this pull request Jul 14, 2026
brew generate-vulns-advisories preserves published/ranges from existing
records and never deletes (Homebrew/brew#23112), so wiping first would
throw away the persisted fixed boundaries and any curated records.
andrew added a commit to Homebrew/advisory-database that referenced this pull request Jul 14, 2026
The workflow refreshes upstream-derived fields but keeps published and
affected[].ranges from disk (Homebrew/brew#23112), so a hand-corrected
fixed boundary survives. Drop the blanket "do not edit by hand".

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates Homebrew’s OSV advisory exporter (Homebrew::Vulns::OsvExport) so it merges generated advisories into an existing target directory rather than overwriting, enabling the advisory database to accumulate records over time while preserving key historical fields.

Changes:

  • Preserve existing on-disk published (when present) and affected[].ranges while regenerating records, and skip rewrites when the record is otherwise unchanged.
  • Avoid stripping previously-enriched fields when the upstream OSV API fetch fails (leave existing enriched records untouched).
  • Add published plus database_specific: { source: "generated" } to generated records, and extend specs to cover merge/no-rewrite behavior and upstream outage behavior.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
Library/Homebrew/vulns/osv_export.rb Implement merge-in-place behavior, upstream-outage handling, and add published/database_specific fields.
Library/Homebrew/test/vulns/osv_export_spec.rb Add/adjust tests for the new merge semantics, non-rewrite comparisons, and upstream failure scenarios.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread Library/Homebrew/vulns/osv_export.rb
Comment thread Library/Homebrew/vulns/osv_export.rb Outdated
andrew added 2 commits July 14, 2026 12:37
When migrating a record written before `published` existed, fall back to
its `modified` timestamp rather than today's so first-rewrite doesn't
jump the publication date forward.

Narrow the upstream cache value with `is_a?(Hash)` at the call site
instead of `T.cast`, and collapse the failed-fetch guard to a single
`next if` now that reassignment isn't needed.
@andrew andrew added this pull request to the merge queue Jul 14, 2026
Merged via the queue into main with commit 945b48c Jul 14, 2026
43 checks passed
@andrew andrew deleted the vulns-osv-export-merge branch July 14, 2026 21:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants