Skip to content

Fix CVSS v3 and v4 parsing/output in GitHub advisory sync.#1129

Merged
jasnow merged 2 commits into
rubysec:masterfrom
connorshea:fix-cvss-score
Jun 20, 2026
Merged

Fix CVSS v3 and v4 parsing/output in GitHub advisory sync.#1129
jasnow merged 2 commits into
rubysec:masterfrom
connorshea:fix-cvss-score

Conversation

@connorshea

Copy link
Copy Markdown
Contributor

Disclaimer: This change was generated using Claude Code, but fully reviewed, tested, and validated by me. This PR description was written by me.

I noticed that generating the YML files for the recent nokogiri security advisories resulted in no CVSS score when the GHSA only had a CVSS v4 score. After investigating, I discovered that we were using a deprecated field in the GraphQL query on the GitHub API, and it did not return any v4 scores. This fixes that problem by updating the GraphQL query in the sync task and then the code for extracting the scores accordingly.

I can add further tests here if desired, the current tests for this file are fairly sparse so I wasn't sure what the project preferences were.

Fixes #1128.

This change was generated using Claude Code, but fully reviewed, tested, and validated by me. This commit message was written by me.
@jasnow

jasnow commented Jun 19, 2026

Copy link
Copy Markdown
Member

Running the modified script on the whole database changes more
than just the cvss_v3 and cvss_v4 fields.

@connorshea

connorshea commented Jun 19, 2026

Copy link
Copy Markdown
Contributor Author

Hmm, from what I can tell that's just due to re-serializing the files when there were changes, mainly to re-wrap with the 80 characters wrapping we have. If you change anything about the returned format (adding a new field, etc.) all of those same extra changes would happen automatically no matter what, regardless of this PR's changes.

Should I commit those changes to this branch for consistency, or is there anything specific in the extra changes that we should figure out how to avoid?

EDIT: Actually I guess some wrap at 90 instead of 80, I'm not totally sure why, I don't think my changes would cause that?

@jasnow

jasnow commented Jun 19, 2026

Copy link
Copy Markdown
Member

My preference is to have no regressions and the "Catch Up" changes to the
advisories from this PR in this PR so it could be easily reviewed.

@connorshea

Copy link
Copy Markdown
Contributor Author

👍 I've updated all the advisories that were missing their CVSS v4 scores to include them now, so re-running the rake task should result in 0 further changes to these advisories.

The only thing I intentionally excluded was net-new advisories, as those I assume should go in their own PRs.

@jasnow jasnow self-requested a review June 19, 2026 18:37

@jasnow jasnow left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  1. Checked out connorshea-fix-cvss-score branch.
  2. Pulled connorshea's change to branch.
  3. Ran "git diff" and find only "cvss_v4" lines and changes to lib/github_advisory_sync.rb file.
  4. Looks good to me.
  5. I will probably want to merge this in a hour so we get its benefit for the 18 advisories that arrived today and 8 yesterday.

@jasnow

jasnow commented Jun 20, 2026

Copy link
Copy Markdown
Member

Forgot - Thanks @connorshea for your contribution.

@jasnow jasnow merged commit c9864f1 into rubysec:master Jun 20, 2026
2 checks passed
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.

Update rake task to pull in CVSS v4 scores from GraphQL API

2 participants