Skip to content

feat(local): support resolving git packages that have a version specified #2209

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Sep 30, 2025
Merged

feat(local): support resolving git packages that have a version specified #2209

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0a9ca7d
test: add case for checking git packages when offline
G-Rath Aug 27, 2025
2cbcb25
feat: support checking git packages when offline
G-Rath Aug 27, 2025
acb8a3b
test: expand case
G-Rath Aug 27, 2025
8b2a2ef
chore: update snapshot
G-Rath Sep 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 53 additions & 0 deletions cmd/osv-scanner/scan/source/__snapshots__/command_test.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1697,6 +1697,59 @@ Total 0 packages affected by 0 known vulnerabilities (0 Critical, 0 High, 0 Medi

---

[TestCommand_CommitSupport/offline_uses_git_tags - 1]
Scanned <rootdir>/testdata/locks-git/osv-scanner.json file as a osv-scanner and found 4 packages
Skipping commit scanning for: 45fda76bc1b9fd74d10e85e0ce9b65a12dcc58b0
Loaded GIT local db from <tempdir>/osv-scanner/GIT/all.zip
Total 2 packages affected by 6 known vulnerabilities (2 Critical, 2 High, 0 Medium, 0 Low, 2 Unknown) from 1 ecosystem.
0 vulnerabilities can be fixed.


+--------------------------------+------+-----------+--------------------------+--------------------------+---------------+-------------------------------------+
| OSV URL | CVSS | ECOSYSTEM | PACKAGE | VERSION | FIXED VERSION | SOURCE |
+--------------------------------+------+-----------+--------------------------+--------------------------+---------------+-------------------------------------+
| https://osv.dev/CVE-2016-2183 | 7.5 | GIT | https://github.com/openssl/openssl@aea7aaf2 | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2025-4575 | | GIT | https://github.com/openssl/openssl@aea7aaf2 | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2016-10931 | 8.1 | GIT | https://github.com/sfackler/rust-openssl@0f428d19 | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2018-20997 | 9.8 | GIT | https://github.com/sfackler/rust-openssl@0f428d19 | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2023-53159 | 9.1 | GIT | https://github.com/sfackler/rust-openssl@0f428d19 | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2025-3416 | | GIT | https://github.com/sfackler/rust-openssl@0f428d19 | -- | testdata/locks-git/osv-scanner.json |
+--------------------------------+------+-----------+-----------------------------------------------------+---------------+-------------------------------------+

---

[TestCommand_CommitSupport/offline_uses_git_tags - 2]

---

[TestCommand_CommitSupport/online_uses_git_commits - 1]
Scanned <rootdir>/testdata/locks-git/osv-scanner.json file as a osv-scanner and found 4 packages
Total 3 packages affected by 11 known vulnerabilities (3 Critical, 1 High, 2 Medium, 0 Low, 5 Unknown) from 1 ecosystem.
0 vulnerabilities can be fixed.


+--------------------------------+------+-----------+----------------------------+-----------------------------+---------------+-------------------------------------+
| OSV URL | CVSS | ECOSYSTEM | PACKAGE | VERSION | FIXED VERSION | SOURCE |
+--------------------------------+------+-----------+----------------------------+-----------------------------+---------------+-------------------------------------+
| https://osv.dev/CVE-2024-12797 | | GIT | https://github.com/openssl/openssl@45fda76b | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2024-13176 | | GIT | https://github.com/openssl/openssl@45fda76b | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2024-9143 | | GIT | https://github.com/openssl/openssl@45fda76b | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2023-53159 | 9.1 | GIT | https://github.com/sfackler-fork/rust-openssl@3b064fdb | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2023-6180 | 5.3 | GIT | https://github.com/sfackler-fork/rust-openssl@3b064fdb | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2025-3416 | | GIT | https://github.com/sfackler-fork/rust-openssl@3b064fdb | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2016-10931 | 8.1 | GIT | https://github.com/sfackler/rust-openssl@0f428d19 | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2018-20997 | 9.8 | GIT | https://github.com/sfackler/rust-openssl@0f428d19 | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2023-53159 | 9.1 | GIT | https://github.com/sfackler/rust-openssl@0f428d19 | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2023-6180 | 5.3 | GIT | https://github.com/sfackler/rust-openssl@0f428d19 | -- | testdata/locks-git/osv-scanner.json |
| https://osv.dev/CVE-2025-3416 | | GIT | https://github.com/sfackler/rust-openssl@0f428d19 | -- | testdata/locks-git/osv-scanner.json |
+--------------------------------+------+-----------+----------------------------------------------------------+---------------+-------------------------------------+

---

[TestCommand_CommitSupport/online_uses_git_commits - 2]

---

[TestCommand_ExplicitExtractors_WithDefaults/empty_plugins_flag_does_nothing - 1]

---
Expand Down
25 changes: 25 additions & 0 deletions cmd/osv-scanner/scan/source/command_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,6 +972,31 @@ func TestCommand_LocalDatabases_AlwaysOffline(t *testing.T) {
}
}

func TestCommand_CommitSupport(t *testing.T) {
t.Parallel()

tests := []testcmd.Case{
{
Name: "online_uses_git_commits",
Args: []string{"", "source", "--lockfile", "osv-scanner:./testdata/locks-git/osv-scanner.json"},
Exit: 1,
},
{
Name: "offline_uses_git_tags",
Args: []string{"", "source", "--offline", "--download-offline-databases", "--lockfile", "osv-scanner:./testdata/locks-git/osv-scanner.json"},
Exit: 1,
},
}

for _, tt := range tests {
t.Run(tt.Name, func(t *testing.T) {
t.Parallel()

testcmd.RunAndMatchSnapshots(t, tt)
})
}
}

func TestCommand_Licenses(t *testing.T) {
t.Parallel()

Expand Down
40 changes: 40 additions & 0 deletions cmd/osv-scanner/scan/source/testdata/locks-git/osv-scanner.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
{
"results": [
{
"packages": [
{
"//1": "version and commit are the same, so rust-openssl advisories should be reported",
"//2": "note online reports CVE-2023-6180 as well due to matching commits for an implicit fork",
"package": {
"name": "https://github.com/sfackler/rust-openssl",
"commit": "0f428d190410263e4daa65b917c0e84707a9c0ef",
"version": "openssl-v0.8.1"
}
},
{
"//1": "repo is different to the advisory, so only online checking will report anything",
"package": {
"name": "https://github.com/sfackler-fork/rust-openssl",
"commit": "3b064fdb022912bbb98f5b8d9d111aeb6fec8f79",
"version": "openssl-v0.10.23"
}
},
{
"//1": "no version, so only online checking will report anything",
"package": {
"name": "https://github.com/openssl/openssl",
"commit": "45fda76bc1b9fd74d10e85e0ce9b65a12dcc58b0"
}
},
{
"//1": "version is for 3.5.0 which is vulnerable to CVE-2025-3416, but commit is for 3.5.1 which is not",
"package": {
"name": "https://github.com/openssl/openssl",
"commit": "aea7aaf2abb04789f5868cbabec406ea43aa84bf",
"version": "openssl-3.5.0"
}
}
]
}
]
}
38 changes: 23 additions & 15 deletions internal/clients/clientimpl/localmatcher/localmatcher.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,29 @@ func (matcher *LocalMatcher) MatchVulnerabilities(ctx context.Context, invs []*e
}

pkg := imodels.FromInventory(inv)
eco := pkg.Ecosystem().Ecosystem

if pkg.Ecosystem().IsEmpty() {
if pkg.Commit() == "" {
// This should never happen, as those results will be filtered out before matching
return nil, errors.New("ecosystem is empty and there is no commit hash")
}

// Is a commit based query, skip local scanning
results = append(results, []*osvschema.Vulnerability{})
// TODO (V2 logging):
cmdlogger.Infof("Skipping commit scanning for: %s", pkg.Commit())
// matching ecosystem-less versions can only be attempted if we have a version
if pkg.Version() == "" {
// Is a commit based query, skip local scanning
results = append(results, []*osvschema.Vulnerability{})

continue
// TODO (V2 logging):
cmdlogger.Infof("Skipping commit scanning for: %s", pkg.Commit())

continue
}

eco = "GIT"
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm surprised this just compiles without casting?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ha yeah I was surprised too - technically it makes sense because the type is a string rather than a "enum" even though it gets used in that fashion, so I'm guessing this is triggering an implicit cast that succeeds because the right hand side is a string 🤷

(in fact, if I add the cast it gets marked as redundant)

}

db, err := matcher.loadDBFromCache(ctx, pkg.Ecosystem())
db, err := matcher.loadDBFromCache(ctx, eco)

if err != nil {
// no logging here as the loader will have already done that
Expand All @@ -87,32 +95,32 @@ func (matcher *LocalMatcher) MatchVulnerabilities(ctx context.Context, invs []*e
// LoadEcosystem tries to preload the ecosystem into the cache, and returns an error if the ecosystem
// cannot be loaded.
func (matcher *LocalMatcher) LoadEcosystem(ctx context.Context, eco osvecosystem.Parsed) error {
_, err := matcher.loadDBFromCache(ctx, eco)
_, err := matcher.loadDBFromCache(ctx, eco.Ecosystem)

return err
}

func (matcher *LocalMatcher) loadDBFromCache(ctx context.Context, eco osvecosystem.Parsed) (*ZipDB, error) {
if db, ok := matcher.dbs[eco.Ecosystem]; ok {
func (matcher *LocalMatcher) loadDBFromCache(ctx context.Context, eco osvschema.Ecosystem) (*ZipDB, error) {
if db, ok := matcher.dbs[eco]; ok {
return db, nil
}

if matcher.failedDBs[eco.Ecosystem] != nil {
return nil, matcher.failedDBs[eco.Ecosystem]
if matcher.failedDBs[eco] != nil {
return nil, matcher.failedDBs[eco]
}

db, err := NewZippedDB(ctx, matcher.dbBasePath, string(eco.Ecosystem), fmt.Sprintf("%s/%s/all.zip", zippedDBRemoteHost, eco.Ecosystem), matcher.userAgent, !matcher.downloadDB)
db, err := NewZippedDB(ctx, matcher.dbBasePath, string(eco), fmt.Sprintf("%s/%s/all.zip", zippedDBRemoteHost, eco), matcher.userAgent, !matcher.downloadDB)

if err != nil {
matcher.failedDBs[eco.Ecosystem] = err
cmdlogger.Errorf("could not load db for %s ecosystem: %v", eco.Ecosystem, err)
matcher.failedDBs[eco] = err
cmdlogger.Errorf("could not load db for %s ecosystem: %v", eco, err)

return nil, err
}

cmdlogger.Infof("Loaded %s local db from %s", db.Name, db.StoredAt)

matcher.dbs[eco.Ecosystem] = db
matcher.dbs[eco] = db

return db, nil
}
Expand Down
18 changes: 18 additions & 0 deletions internal/utility/vulns/vulnerability.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,8 +125,26 @@ func AffectsEcosystem(v osvschema.Vulnerability, ecosystemAffected osvecosystem.
return false
}

func hasGitRangeForRepo(affected osvschema.Affected, repo string) bool {
for _, r := range affected.Ranges {
if r.Type == "GIT" && r.Repo == repo {
return true
}
}

return false
}

func IsAffected(v osvschema.Vulnerability, pkg imodels.PackageInfo) bool {
for _, affected := range v.Affected {
// assume we're dealing with a git-source package whose name is the git repository, and that the version is the tag
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it be clearer for this comment to be within the If statement? Right now it reads like the if statement is using the assumption, when I guess it's the other way around.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Personally I tend to expect comments to be about the statement they precede, and that that tends to be the most common style - there's a number of small factors for that such as the comment is usually explaining the condition rather than the implementation, and editor folding of the content in the braces hides the comment which I find is typically counter to having the comment.

In this case, most of the comment is describing the top if condition, ending with describing what the condition being satisfied means we can do (which assumingly then takes place in the body), so I think it's valid where it is

// the underlying commit has been resolved to (somehow), meaning we can check if it's in the versions listed by the advisory
if pkg.Ecosystem().IsEmpty() && pkg.Commit() != "" && pkg.Version() != "" {
if hasGitRangeForRepo(affected, pkg.Name()) && slices.Contains(affected.Versions, pkg.Version()) {
return true
}
}

// Assume vulnerability has already been validated
if osvecosystem.MustParse(affected.Package.Ecosystem).Equal(pkg.Ecosystem()) &&
affected.Package.Name == pkg.Name() {
Expand Down
Loading