Skip to content
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

Jump to bottom

fix(offline): report all ecosystems without local databases in one single line #1279

Merged
merged 2 commits into from
Sep 30, 2024
Merged

fix(offline): report all ecosystems without local databases in one single line #1279

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2cb6598
test: add case
G-Rath Sep 25, 2024
c04e4c3
fix(offline): report all ecosystems without local databases in one si…
G-Rath Sep 25, 2024
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
42 changes: 42 additions & 0 deletions cmd/osv-scanner/__snapshots__/main_test.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2091,6 +2091,48 @@ databases can only be downloaded when running in offline mode

---

[TestRun_LocalDatabases_AlwaysOffline/#00 - 1]
Scanning dir ./fixtures/locks-requirements
Scanned <rootdir>/fixtures/locks-requirements/my-requirements.txt file and found 1 package
Scanned <rootdir>/fixtures/locks-requirements/requirements-dev.txt file and found 1 package
Scanned <rootdir>/fixtures/locks-requirements/requirements.prod.txt file and found 1 package
Scanned <rootdir>/fixtures/locks-requirements/requirements.txt file and found 3 packages
Scanned <rootdir>/fixtures/locks-requirements/the_requirements_for_test.txt file and found 1 package
Scanning dir ./fixtures/locks-many
Scanned <rootdir>/fixtures/locks-many/Gemfile.lock file and found 1 package
Scanned <rootdir>/fixtures/locks-many/alpine.cdx.xml as CycloneDX SBOM and found 14 packages
Scanned <rootdir>/fixtures/locks-many/composer.lock file and found 1 package
Scanned <rootdir>/fixtures/locks-many/package-lock.json file and found 1 package
Scanned <rootdir>/fixtures/locks-many/yarn.lock file and found 1 package

---

[TestRun_LocalDatabases_AlwaysOffline/#00 - 2]
could not find local databases for ecosystems: Alpine, Packagist, PyPI, RubyGems, npm

---

[TestRun_LocalDatabases_AlwaysOffline/#00 - 3]
Scanning dir ./fixtures/locks-requirements
Scanned <rootdir>/fixtures/locks-requirements/my-requirements.txt file and found 1 package
Scanned <rootdir>/fixtures/locks-requirements/requirements-dev.txt file and found 1 package
Scanned <rootdir>/fixtures/locks-requirements/requirements.prod.txt file and found 1 package
Scanned <rootdir>/fixtures/locks-requirements/requirements.txt file and found 3 packages
Scanned <rootdir>/fixtures/locks-requirements/the_requirements_for_test.txt file and found 1 package
Scanning dir ./fixtures/locks-many
Scanned <rootdir>/fixtures/locks-many/Gemfile.lock file and found 1 package
Scanned <rootdir>/fixtures/locks-many/alpine.cdx.xml as CycloneDX SBOM and found 14 packages
Scanned <rootdir>/fixtures/locks-many/composer.lock file and found 1 package
Scanned <rootdir>/fixtures/locks-many/package-lock.json file and found 1 package
Scanned <rootdir>/fixtures/locks-many/yarn.lock file and found 1 package

---

[TestRun_LocalDatabases_AlwaysOffline/#00 - 4]
could not find local databases for ecosystems: Alpine, Packagist, PyPI, RubyGems, npm

---

[TestRun_LockfileWithExplicitParseAs/#00 - 1]

---
Expand Down
29 changes: 29 additions & 0 deletions cmd/osv-scanner/main_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -641,6 +641,35 @@ func TestRun_LocalDatabases(t *testing.T) {
}
}

func TestRun_LocalDatabases_AlwaysOffline(t *testing.T) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: I've had to put this in a dedicated test because our other local db tests try to reuse the database as an optimization when run locally, which is exactly what we don't want here

t.Parallel()

tests := []cliTestCase{
// a bunch of different lockfiles and ecosystem
{
name: "",
args: []string{"", "--config=./fixtures/osv-scanner-empty-config.toml", "--experimental-offline", "./fixtures/locks-requirements", "./fixtures/locks-many"},
exit: 127,
},
}

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

testDir := testutility.CreateTestDir(t)
old := tt.args
tt.args = []string{"", "--experimental-local-db-path", testDir}
tt.args = append(tt.args, old[1:]...)

// run each test twice since they should provide the same output,
// and the second run should be fast as the db is already available
testCli(t, tt)
testCli(t, tt)
})
}
}

func TestRun_Licenses(t *testing.T) {
t.Parallel()
tests := []cliTestCase{
Expand Down
21 changes: 19 additions & 2 deletions internal/local/check.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ import (
"fmt"
"os"
"path"
"slices"
"strings"

"github.com/google/osv-scanner/pkg/lockfile"
"github.com/google/osv-scanner/pkg/models"
Expand Down Expand Up @@ -116,6 +118,9 @@ func MakeRequest(r reporter.Reporter, query osv.BatchedQuery, offline bool, loca
return db, nil
}

// slice to track ecosystems that did not have an offline database available
var missingDbs []string

for _, query := range query.Queries {
pkg, err := toPackageDetails(query)

Expand Down Expand Up @@ -143,8 +148,13 @@ func MakeRequest(r reporter.Reporter, query osv.BatchedQuery, offline bool, loca
db, err := loadDBFromCache(pkg.Ecosystem)

if err != nil {
// currently, this will actually only error if the PURL cannot be parses
r.Errorf("could not load db for %s ecosystem: %v\n", pkg.Ecosystem, err)
if errors.Is(err, ErrOfflineDatabaseNotFound) {
missingDbs = append(missingDbs, string(pkg.Ecosystem))
} else {
// the most likely error at this point is that the PURL could not be parsed
r.Errorf("could not load db for %s ecosystem: %v\n", pkg.Ecosystem, err)
Comment on lines +154 to +155
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

note: this no longer has coverage, but that's fine

while my comment mentions a possible error being a PURL could not be parsed, we've not got a test for that and I'm not sure how easy it'll be - I've made a note to look into that as a separate thing, but we shouldn't let it block this as I think we want to be playing it safe i.e. by logging the error rather than assume the only possible error could be that the database cannot be found

}

results = append(results, osv.Response{Vulns: []models.Vulnerability{}})

continue
Expand All @@ -153,5 +163,12 @@ func MakeRequest(r reporter.Reporter, query osv.BatchedQuery, offline bool, loca
results = append(results, osv.Response{Vulns: db.VulnerabilitiesAffectingPackage(pkg)})
}

if len(missingDbs) > 0 {
missingDbs = slices.Compact(missingDbs)
slices.Sort(missingDbs)

r.Errorf("could not find local databases for ecosystems: %s\n", strings.Join(missingDbs, ", "))
}

return &osv.HydratedBatchedResponse{Results: results}, nil
}