chore(trivy): update trivy version and enforce OCI compliant repo names in local image storage (#1068)

1. chore(trivy): update trivy library version

The trivy team switched github.com/urfave/cli for viper so
there are some other code changes as well.

Since we don't use github.com/urfave/cli directly in our software
we needed to add a tools.go in order for "go mod tidy" to not delete it.
See this pattern explained in:
- https://github.com/99designs/gqlgen#quick-start
- https://github.com/golang/go/wiki/Modules#how-can-i-track-tool-dependencies-for-a-module
- https://github.com/go-modules-by-example/index/blob/master/010_tools/README.md#walk-through

The jobs using "go get -u" have been updated to use "go install", since go get
modifies the go.mod by upgrading some of the packages, but downgrading trivy to an older
version with broken dependencies

2. fix(storage) Update local storage to ignore folder names not compliant with dist spec
Also updated trivy to download the DB and cache results under the rootDir/_trivy folder

3. fix(s3): one of the s3 tests was missing the skipIt call
This caused a failure when running locally without s3 being available

4. make sure the offline scanning is enabled, and zot only downloads the trivy DB
on the regular schedule, and doesn't download the DB on every image scan

ci: increase build and test timeout as tests are reaching the limit more often

Signed-off-by: Andrei Aaron <aaaron@luxoft.com>

pkg/storage/local/local.go

@@ -31,6 +31,7 @@ import (
 	"zotregistry.io/zot/pkg/common"
 	"zotregistry.io/zot/pkg/extensions/monitoring"
 	zlog "zotregistry.io/zot/pkg/log"
+	zreg "zotregistry.io/zot/pkg/regexp"
 	"zotregistry.io/zot/pkg/scheduler"
 	"zotregistry.io/zot/pkg/storage"
 	"zotregistry.io/zot/pkg/storage/cache"
@@ -154,6 +155,12 @@ func (is *ImageStoreLocal) initRepo(name string) error {
 		return zerr.ErrInvalidRepositoryName
 	}
 
+	if !zreg.FullNameRegexp.MatchString(name) {
+		is.log.Error().Str("repo", name).Msg("invalid repository name")
+
+		return zerr.ErrInvalidRepositoryName
+	}
+
 	// create "blobs" subdir
 	err := ensureDir(path.Join(repoDir, "blobs"), is.log)
 	if err != nil {
@@ -221,6 +228,10 @@ func (is *ImageStoreLocal) ValidateRepo(name string) (bool, error) {
 	// https://github.com/opencontainers/image-spec/blob/master/image-layout.md#content
 	// at least, expect at least 3 entries - ["blobs", "oci-layout", "index.json"]
 	// and an additional/optional BlobUploadDir in each image store
+	if !zreg.FullNameRegexp.MatchString(name) {
+		return false, zerr.ErrInvalidRepositoryName
+	}
+
 	dir := path.Join(is.rootDir, name)
 	if !is.DirExists(dir) {
 		return false, zerr.ErrRepoNotFound

pkg/storage/local/local_test.go

@@ -30,6 +30,7 @@ import (
 	"zotregistry.io/zot/pkg/log"
 	"zotregistry.io/zot/pkg/storage"
 	"zotregistry.io/zot/pkg/storage/cache"
+	storageConstants "zotregistry.io/zot/pkg/storage/constants"
 	"zotregistry.io/zot/pkg/storage/local"
 	"zotregistry.io/zot/pkg/test"
 )
@@ -1326,6 +1327,11 @@ func TestNegativeCases(t *testing.T) {
 		// Init repo should fail if repo is invalid UTF-8
 		err = imgStore.InitRepo("hi \255")
 		So(err, ShouldNotBeNil)
+
+		// Init repo should fail if repo name does not match spec
+		err = imgStore.InitRepo("_trivy")
+		So(err, ShouldNotBeNil)
+		So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
 	})
 
 	Convey("Invalid validate repo", t, func(c C) {
@@ -1430,7 +1436,7 @@ func TestNegativeCases(t *testing.T) {
 			So(func() { _, _ = imgStore.ValidateRepo("test") }, ShouldPanic)
 		}
 
-		err = os.Chmod(dir, 0o755) // remove all perms
+		err = os.Chmod(dir, 0o755) // add perms
 		if err != nil {
 			panic(err)
 		}
@@ -2483,10 +2489,53 @@ func TestValidateRepo(t *testing.T) {
 		_, err = imgStore.ValidateRepo("test-dir")
 		So(err, ShouldNotBeNil)
 	})
+
+	Convey("Get error when repo name is not compliant with repo spec", t, func() {
+		dir := t.TempDir()
+
+		log := log.Logger{Logger: zerolog.New(os.Stdout)}
+		metrics := monitoring.NewMetricsServer(false, log)
+		cacheDriver, _ := storage.Create("boltdb", cache.BoltDBDriverParameters{
+			RootDir:     dir,
+			Name:        "cache",
+			UseRelPaths: true,
+		}, log)
+		imgStore := local.NewImageStore(dir, true, storage.DefaultGCDelay,
+			true, true, log, metrics, nil, cacheDriver)
+
+		_, err := imgStore.ValidateRepo(".")
+		So(err, ShouldNotBeNil)
+		So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+		_, err = imgStore.ValidateRepo("..")
+		So(err, ShouldNotBeNil)
+		So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+		err = os.Mkdir(path.Join(dir, "_test-dir"), 0o755)
+		So(err, ShouldBeNil)
+
+		_, err = imgStore.ValidateRepo("_test-dir")
+		So(err, ShouldNotBeNil)
+		So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+		err = os.Mkdir(path.Join(dir, ".test-dir"), 0o755)
+		So(err, ShouldBeNil)
+
+		_, err = imgStore.ValidateRepo(".test-dir")
+		So(err, ShouldNotBeNil)
+		So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+		err = os.Mkdir(path.Join(dir, "-test-dir"), 0o755)
+		So(err, ShouldBeNil)
+
+		_, err = imgStore.ValidateRepo("-test-dir")
+		So(err, ShouldNotBeNil)
+		So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+	})
 }
 
-func TestGetRepositoriesError(t *testing.T) {
-	Convey("Get error when returning relative path", t, func() {
+func TestGetRepositories(t *testing.T) {
+	Convey("Verify errors and repos returned by GetRepositories()", t, func() {
 		dir := t.TempDir()
 
 		log := log.Logger{Logger: zerolog.New(os.Stdout)}
@@ -2500,15 +2549,178 @@ func TestGetRepositoriesError(t *testing.T) {
 			true, true, log, metrics, nil, cacheDriver,
 		)
 
-		// create valid directory with permissions
-		err := os.Mkdir(path.Join(dir, "test-dir"), 0o755)
+		// Create valid directory with permissions
+		err := os.Mkdir(path.Join(dir, "test-dir"), 0o755) //nolint: gosec
 		So(err, ShouldBeNil)
 
-		err = os.WriteFile(path.Join(dir, "test-dir/test-file"), []byte("this is test file"), 0o000)
+		err = os.WriteFile(path.Join(dir, "test-dir/test-file"), []byte("this is test file"), 0o755) //nolint: gosec
 		So(err, ShouldBeNil)
 
-		_, err = imgStore.GetRepositories()
+		// Folder is not a repo as it is missing the requires files/subfolder
+		repos, err := imgStore.GetRepositories()
 		So(err, ShouldBeNil)
+		So(len(repos), ShouldEqual, 0)
+
+		il := ispec.ImageLayout{Version: ispec.ImageLayoutVersion}
+		layoutFileContent, err := json.Marshal(il)
+		So(err, ShouldBeNil)
+
+		// Folder becomes a repo after the missing content is added
+		err = os.Mkdir(path.Join(dir, "test-dir", "blobs"), 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.Mkdir(path.Join(dir, "test-dir", storageConstants.BlobUploadDir), 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.WriteFile(path.Join(dir, "test-dir", "index.json"), []byte{}, 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.WriteFile(path.Join(dir, "test-dir", ispec.ImageLayoutFile), layoutFileContent, 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		// Verify the new repo is turned
+		repos, err = imgStore.GetRepositories()
+		So(err, ShouldBeNil)
+		So(len(repos), ShouldEqual, 1)
+		So(repos[0], ShouldEqual, "test-dir")
+
+		// create directory starting with underscore, which is not OCI a dist spec compliant repo name
+		// [a-z0-9]+([._-][a-z0-9]+)*(/[a-z0-9]+([._-][a-z0-9]+)*)*
+		err = os.MkdirAll(path.Join(dir, "_trivy", "db"), 0o755)
+		So(err, ShouldBeNil)
+
+		err = os.WriteFile(path.Join(dir, "_trivy", "db", "trivy.db"), []byte("this is test file"), 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		// Folder with invalid name is not a repo as it is missing the requires files/subfolder
+		repos, err = imgStore.GetRepositories()
+		So(err, ShouldBeNil)
+		So(len(repos), ShouldEqual, 1)
+		So(repos[0], ShouldEqual, "test-dir")
+
+		// Add missing content to folder with invalid name
+		err = os.Mkdir(path.Join(dir, "_trivy", "blobs"), 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.Mkdir(path.Join(dir, "_trivy", storageConstants.BlobUploadDir), 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.WriteFile(path.Join(dir, "_trivy", "index.json"), []byte{}, 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.WriteFile(path.Join(dir, "_trivy", ispec.ImageLayoutFile), layoutFileContent, 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		// Folder with invalid name doesn't become a repo after the missing content is added
+		repos, err = imgStore.GetRepositories()
+		So(err, ShouldBeNil)
+		t.Logf("repos %v", repos)
+		So(len(repos), ShouldEqual, 1)
+		So(repos[0], ShouldEqual, "test-dir")
+
+		// Rename folder with invalid name to a valid one
+		err = os.Rename(path.Join(dir, "_trivy"), path.Join(dir, "test-dir-2"))
+		So(err, ShouldBeNil)
+
+		// Verify both repos are now visible
+		repos, err = imgStore.GetRepositories()
+		So(err, ShouldBeNil)
+		t.Logf("repos %v", repos)
+		So(len(repos), ShouldEqual, 2)
+		So(repos, ShouldContain, "test-dir")
+		So(repos, ShouldContain, "test-dir-2")
+	})
+
+	Convey("Verify GetRepositories() doesn't return '.' when having an oci layout as root directory ", t, func() {
+		dir := t.TempDir()
+
+		log := log.Logger{Logger: zerolog.New(os.Stdout)}
+		metrics := monitoring.NewMetricsServer(false, log)
+		cacheDriver, _ := storage.Create("boltdb", cache.BoltDBDriverParameters{
+			RootDir:     dir,
+			Name:        "cache",
+			UseRelPaths: true,
+		}, log)
+
+		imgStore := local.NewImageStore(dir, true, storage.DefaultGCDelay,
+			true, true, log, metrics, nil, cacheDriver,
+		)
+
+		// Root dir does not contain repos
+		repos, err := imgStore.GetRepositories()
+		So(err, ShouldBeNil)
+		So(len(repos), ShouldEqual, 0)
+
+		// Configure root directory as an oci layout
+		err = os.Mkdir(path.Join(dir, "blobs"), 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.Mkdir(path.Join(dir, storageConstants.BlobUploadDir), 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.WriteFile(path.Join(dir, "index.json"), []byte{}, 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		il := ispec.ImageLayout{Version: ispec.ImageLayoutVersion}
+		layoutFileContent, err := json.Marshal(il)
+		So(err, ShouldBeNil)
+
+		err = os.WriteFile(path.Join(dir, ispec.ImageLayoutFile), layoutFileContent, 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		// Verify root directory is not returned as a repo
+		repos, err = imgStore.GetRepositories()
+		So(err, ShouldBeNil)
+		t.Logf("repos %v", repos)
+		So(len(repos), ShouldEqual, 0)
+	})
+
+	Convey("Verify GetRepositories() doesn't return '..'", t, func() {
+		dir := t.TempDir()
+
+		rootDir := path.Join(dir, "rootDir")
+		err := os.Mkdir(rootDir, 0o755)
+		So(err, ShouldBeNil)
+
+		log := log.Logger{Logger: zerolog.New(os.Stdout)}
+		metrics := monitoring.NewMetricsServer(false, log)
+		cacheDriver, _ := storage.Create("boltdb", cache.BoltDBDriverParameters{
+			RootDir:     rootDir,
+			Name:        "cache",
+			UseRelPaths: true,
+		}, log)
+
+		imgStore := local.NewImageStore(rootDir, true, storage.DefaultGCDelay,
+			true, true, log, metrics, nil, cacheDriver,
+		)
+
+		// Root dir does not contain repos
+		repos, err := imgStore.GetRepositories()
+		So(err, ShouldBeNil)
+		So(len(repos), ShouldEqual, 0)
+
+		// Configure parent of root directory as an oci layout
+		err = os.Mkdir(path.Join(dir, "blobs"), 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.Mkdir(path.Join(dir, storageConstants.BlobUploadDir), 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		err = os.WriteFile(path.Join(dir, "index.json"), []byte{}, 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		il := ispec.ImageLayout{Version: ispec.ImageLayoutVersion}
+		layoutFileContent, err := json.Marshal(il)
+		So(err, ShouldBeNil)
+
+		err = os.WriteFile(path.Join(dir, ispec.ImageLayoutFile), layoutFileContent, 0o755) //nolint: gosec
+		So(err, ShouldBeNil)
+
+		// Verify root directory is not returned as a repo
+		repos, err = imgStore.GetRepositories()
+		So(err, ShouldBeNil)
+		t.Logf("repos %v", repos)
+		So(len(repos), ShouldEqual, 0)
 	})
 }
 

pkg/storage/s3/s3.go

@@ -26,6 +26,7 @@ import (
 	zerr "zotregistry.io/zot/errors"
 	"zotregistry.io/zot/pkg/extensions/monitoring"
 	zlog "zotregistry.io/zot/pkg/log"
+	zreg "zotregistry.io/zot/pkg/regexp"
 	"zotregistry.io/zot/pkg/scheduler"
 	"zotregistry.io/zot/pkg/storage"
 	"zotregistry.io/zot/pkg/storage/cache"
@@ -122,6 +123,12 @@ func (is *ObjectStorage) Unlock(lockStart *time.Time) {
 func (is *ObjectStorage) initRepo(name string) error {
 	repoDir := path.Join(is.rootDir, name)
 
+	if !zreg.FullNameRegexp.MatchString(name) {
+		is.log.Error().Str("repo", name).Msg("invalid repository name")
+
+		return zerr.ErrInvalidRepositoryName
+	}
+
 	// "oci-layout" file - create if it doesn't exist
 	ilPath := path.Join(repoDir, ispec.ImageLayoutFile)
 	if _, err := is.store.Stat(context.Background(), ilPath); err != nil {
@@ -176,6 +183,10 @@ func (is *ObjectStorage) InitRepo(name string) error {
 
 // ValidateRepo validates that the repository layout is complaint with the OCI repo layout.
 func (is *ObjectStorage) ValidateRepo(name string) (bool, error) {
+	if !zreg.FullNameRegexp.MatchString(name) {
+		return false, zerr.ErrInvalidRepositoryName
+	}
+
 	// https://github.com/opencontainers/image-spec/blob/master/image-layout.md#content
 	// at least, expect at least 3 entries - ["blobs", "oci-layout", "index.json"]
 	// and an additional/optional BlobUploadDir in each image store

pkg/storage/s3/s3_test.go

@@ -386,8 +386,8 @@ func TestStorageDriverStatFunction(t *testing.T) {
 	3) the returned storageDriver.FileInfo will report that isDir() is true.
 	*/
 	Convey("Validate storageDriver.Stat() and isDir() functions with zot storage API", t, func(c C) {
-		repo1 := "repo/testImageA"
-		repo2 := "repo/testImage"
+		repo1 := "repo/testimagea"
+		repo2 := "repo/testimage"
 
 		So(imgStore, ShouldNotBeNil)
 
@@ -446,6 +446,8 @@ func TestStorageDriverStatFunction(t *testing.T) {
 }
 
 func TestGetOrasAndOCIReferrers(t *testing.T) {
+	skipIt(t)
+
 	repo := "zot-test"
 
 	uuid, err := guuid.NewV4()
@@ -655,6 +657,50 @@ func TestNegativeCasesObjectsStorage(t *testing.T) {
 		storeDriver, imgStore, _ := createObjectsStore(testDir, tdir, true)
 		defer cleanupStorage(storeDriver, testDir)
 
+		Convey("Invalid repo name", func(c C) {
+			// Validate repo should fail if repo name does not match spec
+			_, err := imgStore.ValidateRepo(".")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+			_, err = imgStore.ValidateRepo("..")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+			_, err = imgStore.ValidateRepo("_test-dir")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+			_, err = imgStore.ValidateRepo(".test-dir")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+			_, err = imgStore.ValidateRepo("-test-dir")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+			// Init repo should fail if repo name does not match spec
+			err = imgStore.InitRepo(".")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+			err = imgStore.InitRepo("..")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+			err = imgStore.InitRepo("_test-dir")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+			err = imgStore.InitRepo(".test-dir")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+
+			err = imgStore.InitRepo("-test-dir")
+			So(err, ShouldNotBeNil)
+			So(errors.Is(err, zerr.ErrInvalidRepositoryName), ShouldBeTrue)
+		})
+
 		Convey("Invalid validate repo", func(c C) {
 			So(imgStore.InitRepo(testImage), ShouldBeNil)
 			objects, err := storeDriver.List(context.Background(), path.Join(imgStore.RootDir(), testImage))