diff --git a/cmd/notation/cert/delete_test.go b/cmd/notation/cert/delete_test.go index 310d1ddaf..3f2c72909 100644 --- a/cmd/notation/cert/delete_test.go +++ b/cmd/notation/cert/delete_test.go @@ -14,8 +14,12 @@ package cert import ( + "os" + "path/filepath" "reflect" "testing" + + "github.com/notaryproject/notation-go/dir" ) func TestCertDeleteCommand(t *testing.T) { @@ -51,3 +55,46 @@ func TestCertDeleteCommand_MissingArgs(t *testing.T) { t.Fatal("Parse Args expected error, but ok") } } + +// A named store of "." or ".." must not resolve to the store type directory +// or to the parent of it, which would delete other trust stores. +func TestCertDelete_DotNamedStoresDoNotTouchOtherStores(t *testing.T) { + for _, namedStore := range []string{".", ".."} { + t.Run(namedStore, func(t *testing.T) { + oldConfigDir := dir.UserConfigDir + t.Cleanup(func() { dir.UserConfigDir = oldConfigDir }) + dir.UserConfigDir = t.TempDir() + + certFiles := make([]string, 0, 2) + for _, store := range [][2]string{{"ca", "store1"}, {"tsa", "store2"}} { + storeDir, err := dir.ConfigFS().SysPath(dir.TrustStoreDir, "x509", store[0], store[1]) + if err != nil { + t.Fatal(err) + } + if err := os.MkdirAll(storeDir, 0o700); err != nil { + t.Fatal(err) + } + certFile := filepath.Join(storeDir, "cert.pem") + if err := os.WriteFile(certFile, []byte("cert"), 0o600); err != nil { + t.Fatal(err) + } + certFiles = append(certFiles, certFile) + } + + err := deleteCerts(&certDeleteOpts{ + storeType: "ca", + namedStore: namedStore, + all: true, + confirmed: true, + }) + if err == nil { + t.Fatalf("deleteCerts() with the named store %q expected an error, but got nil", namedStore) + } + for _, certFile := range certFiles { + if _, statErr := os.Stat(certFile); statErr != nil { + t.Errorf("%s was removed: %v", certFile, statErr) + } + } + }) + } +} diff --git a/cmd/notation/internal/truststore/truststore.go b/cmd/notation/internal/truststore/truststore.go index 881a312a2..c7ff90291 100644 --- a/cmd/notation/internal/truststore/truststore.go +++ b/cmd/notation/internal/truststore/truststore.go @@ -226,7 +226,11 @@ func IsValidStoreType(storeType string) bool { return false } -// IsValidFileName checks if a file name is cross-platform compatible +var validFileName = regexp.MustCompile(`^[a-zA-Z0-9_.-]+$`) + +// IsValidFileName checks if a file name is cross-platform compatible. +// "." and ".." are rejected, because they refer to a directory and not to a +// file name inside of one. func IsValidFileName(fileName string) bool { - return regexp.MustCompile(`^[a-zA-Z0-9_.-]+$`).MatchString(fileName) + return fileName != "." && fileName != ".." && validFileName.MatchString(fileName) } diff --git a/cmd/notation/internal/truststore/truststore_test.go b/cmd/notation/internal/truststore/truststore_test.go index b8aab3635..01cea0fba 100644 --- a/cmd/notation/internal/truststore/truststore_test.go +++ b/cmd/notation/internal/truststore/truststore_test.go @@ -119,3 +119,28 @@ func TestDeleteAllCerts(t *testing.T) { } }) } + +func TestIsValidFileName(t *testing.T) { + tests := []struct { + name string + fileName string + want bool + }{ + {"letters, digits and separators", "ok-1_.x", true}, + {"hidden style name", ".hidden", true}, + {"two dots inside a name", "a..b", true}, + {"empty", "", false}, + {"current directory", ".", false}, + {"parent directory", "..", false}, + {"slash", "a/b", false}, + {"backslash", `a\b`, false}, + {"space", "a b", false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + if got := IsValidFileName(tt.fileName); got != tt.want { + t.Fatalf("IsValidFileName(%q) = %v, want %v", tt.fileName, got, tt.want) + } + }) + } +}