Skip to content

feat: show fixed version for vulnerabilities#967

Merged
danielroe merged 9 commits intonpmx-dev:mainfrom
Flo0806:feat/fixed-vulns-versions
Feb 9, 2026
Merged

feat: show fixed version for vulnerabilities#967
danielroe merged 9 commits intonpmx-dev:mainfrom
Flo0806:feat/fixed-vulns-versions

Conversation

@Flo0806
Copy link
Copy Markdown
Contributor

@Flo0806 Flo0806 commented Feb 4, 2026

Fixes: #954


Get OSV-fixed version(s) for vulnerabilities if available and shoe it inside vulnerabilities banner.

@vercel
Copy link
Copy Markdown
Contributor

vercel Bot commented Feb 4, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
npmx.dev Ready Ready Preview, Comment Feb 9, 2026 9:05am
2 Skipped Deployments
Project Deployment Actions Updated (UTC)
docs.npmx.dev Ignored Ignored Preview Feb 9, 2026 9:05am
npmx-lunaria Ignored Ignored Feb 9, 2026 9:05am

Request Review

@github-actions
Copy link
Copy Markdown

github-actions Bot commented Feb 4, 2026

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

File Note
lunaria/files/de-DE.json Localization changed, will be marked as complete. 🔄️
lunaria/files/en-GB.json Localization changed, will be marked as complete. 🔄️
lunaria/files/en-US.json Source changed, localizations will be marked as outdated.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 4, 2026

Codecov Report

❌ Patch coverage is 0% with 3 lines in your changes missing coverage. Please review.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
app/components/Package/VulnerabilityTree.vue 0.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Feb 4, 2026

📝 Walkthrough

Walkthrough

Adds UI rendering of a vulnerability's fixed version next to each vulnerability line in the Package component, with a localized tooltip and link to the package route for that version. Backend logic parses OSV affected ranges, evaluates semver intervals to compute an optional fixedIn value per vulnerability, and includes it in VulnerabilitySummary. New OSV-related types (range, event, affected) and VulnerabilitySummary.fixedIn were exported. Locale keys for the tooltip were added and unit tests cover fixedIn extraction, including prerelease and multiple-interval cases.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description check ✅ Passed The PR description directly references issue #954 and describes the implementation intent to retrieve and display fixed versions for vulnerabilities in the banner.
Linked Issues check ✅ Passed The implementation successfully addresses issue #954 requirements: adds fixedIn field extraction in dependency-analysis, updates type definitions, extends UI to display fixed versions in VulnerabilityTree component, includes comprehensive tests, and handles prerelease/range edge cases.
Out of Scope Changes check ✅ Passed All changes remain scoped to implementing the fixed-version feature: backend logic, type definitions, frontend UI, internationalisation strings, and corresponding tests. No unrelated modifications detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

No actionable comments were generated in the recent review. 🎉


Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
shared/types/dependency-analysis.ts (1)

39-83: ⚠️ Potential issue | 🟠 Major

Type definitions do not enforce OSV schema constraints; strict type-safety required.

The OsvRangeEvent interface allows zero or multiple fields simultaneously, but the official OSV schema mandates exactly one of: introduced, fixed, last_affected, or limit. Additionally, OsvRange is missing the required repo field (mandatory when type is 'GIT'), and the events array constraint—at least one introduced event must be present—is not enforced. Use a discriminated union or more precise typing to reflect these constraints and ensure strict type-safety at the API boundary per coding guidelines.

Comment thread server/utils/dependency-analysis.ts
Copy link
Copy Markdown
Member

@danielroe danielroe left a comment

Choose a reason for hiding this comment

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

in general I think we should avoid creating ai translations as we currently don't have a way of asking for them to be reviewed by native speakers

@Flo0806
Copy link
Copy Markdown
Contributor Author

Flo0806 commented Feb 5, 2026

in general I think we should avoid creating ai translations as we currently don't have a way of asking for them to be reviewed by native speakers

So I should let translations empty? Or is there a "team" to help? My last request got lost because there was too much information in Discord i18n.

@danielroe
Copy link
Copy Markdown
Member

danielroe commented Feb 5, 2026

yes, just leave the translations empty 🙏

@danielroe danielroe marked this pull request as draft February 5, 2026 09:30
@Flo0806
Copy link
Copy Markdown
Contributor Author

Flo0806 commented Feb 5, 2026

@danielroe now it picks the current next fixed version of a vulnerability. Additional I added some tests.

@Flo0806 Flo0806 marked this pull request as ready for review February 5, 2026 18:04
@trueberryless
Copy link
Copy Markdown
Contributor

Native speaker here, for this one we could keep the German translation as is, it's correct 👍

Copy link
Copy Markdown
Contributor

@trueberryless trueberryless left a comment

Choose a reason for hiding this comment

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

LGTM 🎉

Awesome tests, nice OSV links 🙌

Also looks good:

Image

Comment thread server/utils/dependency-analysis.ts Outdated
Comment on lines +156 to +157
const introduced = range.events.find(e => e.introduced)?.introduced
const fixed = range.events.find(e => e.fixed)?.fixed
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.

could there be multiple of the same kind of event for a given package?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Normally not from what I've read

Copy link
Copy Markdown
Member

@serhalp serhalp Feb 8, 2026

Choose a reason for hiding this comment

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

Comment thread app/components/Package/VulnerabilityTree.vue Outdated
@serhalp
Copy link
Copy Markdown
Member

serhalp commented Feb 9, 2026

👀 #967 (comment)

^ this still seems to not quite behave as one would expect. I think the minimal repro is something like:

  • vulnerable: [>= 3.4.5-foo.2, < 3.4.8, >= 3.5.9-foo.7, < 3.5.9-foo.15]
  • patched: [3.4.8, 3.5.9-foo.15]

In this case if you're looking at e.g. vulnerable 3.4.6 we currently suggest (I think) bumping to patched 3.5.9-foo.15, which is technically not inaccurate, but we should ideally suggest 3.4.8 — or keep it simple and show all options.

@danielroe
Copy link
Copy Markdown
Member

in practice it was the same - it returned the 'first' fix which would have been 3.4.8 in your example (and in the sample OSV data I looked at). but I've pushed a fix to make that explicit

@danielroe danielroe disabled auto-merge February 9, 2026 12:31
@danielroe danielroe merged commit 14449ea into npmx-dev:main Feb 9, 2026
17 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.

feat: show fixed versions for vulnerabilities

5 participants