Skip to content

fix: skip withdrawn OSV advisories in parse_advisory_data_v3#2240

Open
shivamshrma09 wants to merge 1 commit intoaboutcode-org:mainfrom
shivamshrma09:fix-withdrawn-advisory-2238-clean
Open

fix: skip withdrawn OSV advisories in parse_advisory_data_v3#2240
shivamshrma09 wants to merge 1 commit intoaboutcode-org:mainfrom
shivamshrma09:fix-withdrawn-advisory-2238-clean

Conversation

@shivamshrma09
Copy link
Copy Markdown

Summary

Fixes #2238

pkg:pypi/py@1.11.0 was shown as affected in GitHub and GitLab advisories even though the advisory GHSA-w596-4wvx-j9j6 was withdrawn.

Root Cause

parse_advisory_data_v3 in pipes/osv_v2.py had no withdrawn check, so withdrawn advisories were being imported by the v2 pipeline.

Changes

  • pipes/osv_v2.py: Added withdrawn check at the top of parse_advisory_data_v3, consistent with the existing check in parse_advisory_data (v1). This covers all 5 pipelines using this function: github_osv_importer, oss_fuzz, pypa_importer, pysec_importer, ubuntu_osv_importer.
  • importers/osv.py: Added withdrawn check in parse_advisory_data (v1).
  • importers/github_osv.py: Guard yield against None return.
  • tests/pipes/test_osv_v2.py: Added tests for withdrawn and non-withdrawn cases.
  • tests/test_osv.py: Added tests for withdrawn and non-withdrawn cases.

Signed-off-by: shivamshrma09 shivamsharma27107@gmail.com

Withdrawn advisories were being imported by the v2 pipeline because parse_advisory_data_v3 in pipes/osv_v2.py had no withdrawn check. This caused packages like pkg:pypi/py@1.11.0 to appear as affected even though the advisory (e.g. GHSA-w596-4wvx-j9j6) was withdrawn.

Add a withdrawn check at the top of parse_advisory_data_v3, consistent with the existing check in parse_advisory_data (v1). This fix covers all 5 pipelines that use parse_advisory_data_v3: github_osv_importer, oss_fuzz, pypa_importer, pysec_importer, ubuntu_osv_importer.

Add tests for both withdrawn and non-withdrawn cases in test_osv_v2.py.

Fixes aboutcode-org#2238

Signed-off-by: shivamshrma09 <shivamsharma27107@gmail.com>
Copy link
Copy Markdown
Collaborator

@ziadhany ziadhany left a comment

Choose a reason for hiding this comment

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

@shivamshrma09 Please also fix the CI, and don't use AI- generated tools.

advisory_url="https://github.com/github/advisory-database/blob/main/advisories/GHSA-w596-4wvx-j9j6.json",
advisory_text="",
)
assert result is None
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

@shivamshrma09 withdrawn OSV advisories should not affect any package, but I think it should still been stored and reported.

so the result shouldn't be None

Suggested change
assert result is None

@shivamshrma09
Copy link
Copy Markdown
Author

@ziadhany i understand your point. initialy i understand the codebase correctly but can't write a good logic i thought that we should skip the data if it's not affecting any package, but i agree that we should store that data also and add an affected_packages DB filed empty.

i am thinking to add an prefix to summary like
WITHRAWN: ....summary to make it clear.
and i am working on CI fixes

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.

Withdrawn advisory should not affect any package

2 participants