[Cosigned] Glob matching improvement

pkg/apis/config/glob.go

@@ -15,17 +15,53 @@
 
 package config
 
-import "strings"
-
-// GlobMatch takes a string and handles only trailing '*' character as a
-// wildcard. This is little different from various packages that I was able
-// to find, since they handle '*' anywhere in the string as a wildcard. For our
-// use we only want to handle it at the end, and hence this effectively turns
-// into 'hasPrefix' string matching up to the trailing *.
-func GlobMatch(image, glob string) bool {
-	if !strings.HasSuffix(glob, "*") {
-		// Doesn't end with *, so do an exact match
-		return image == glob
+import (
+	"net/url"
+	"path/filepath"
+	"strings"
+)
+
+const (
+	ResolvedDockerhubHost = "index.docker.io/"
+	// Images such as "busybox" reside in the dockerhub "library" repository
+	// The full resolved image reference would be index.docker.io/library/busybox
+	DockerhubPublicRepository = "library/"
+)
+
+// GlobMatch will attempt to:
+// 1. match the glob first
+// 2. When the pattern is <repository>/*, therefore missing a host,
+//    it should match for the resolved image digest in the form of index.docker.io/<repository>/*
+// 3. When the pattern is <image>, it should match for the resolved image digest
+//    against the official Dockerhub repository in the form of index.docker.io/library/*
+func GlobMatch(glob, image string) (bool, error) {
+	matched, err := filepath.Match(glob, image)
+	if err != nil {
+		return false, err
+	}
+
+	// If matched, return early
+	if matched {
+		return matched, nil
+	}
+
+	// If not matched, check if missing host and default to index.docker.io
+	u, err := url.Parse(glob)
+	if err != nil {
+		return false, err
 	}
-	return strings.HasPrefix(image, strings.TrimSuffix(glob, "*"))
+
+	if u.Host == "" {
+		dockerhubGlobPattern := ResolvedDockerhubHost
+
+		// If the image is expected to be part of the Dockerhub official "library" repository
+		if len(strings.Split(u.Path, "/")) < 2 {
+			dockerhubGlobPattern += DockerhubPublicRepository
+		}
+
+		dockerhubGlobPattern += glob
+		return filepath.Match(dockerhubGlobPattern, image)
+	}
+
+	return matched, nil
 }

pkg/apis/config/glob_test.go

@@ -15,26 +15,67 @@
 package config
 
 import (
+	"runtime"
 	"testing"
 )
 
 func TestGlobMatch(t *testing.T) {
 	tests := []struct {
-		glob  string
-		input string
-		match bool
+		glob         string
+		input        string
+		match        bool
+		errString    string
+		windowsMatch *struct {
+			match bool
+		}
 	}{
 		{glob: "foo", input: "foo", match: true},     // exact match
 		{glob: "fooo*", input: "foo", match: false},  // prefix too long
 		{glob: "foo*", input: "foobar", match: true}, // works
 		{glob: "foo*", input: "foo", match: true},    // works
 		{glob: "*", input: "foo", match: true},       // matches anything
 		{glob: "*", input: "bar", match: true},       // matches anything
+		{glob: "*foo*", input: "1foo2", match: true}, // matches wildcard around
+		{glob: "*repository/*", input: "repository/image", match: true},
+		{glob: "*.repository/*", input: "other.repository/image", match: true},
+		{glob: "repository/*", input: "repository/image", match: true},
+		{glob: "repository/*", input: "other.repository/image", match: false},
+		{glob: "repository/*", input: "index.docker.io/repository/image", match: true}, // Testing resolved digest
+		{glob: "image", input: "index.docker.io/library/image", match: true},           // Testing resolved digest and official dockerhub public repository
+		{glob: "[", input: "[", match: false, errString: "syntax error in pattern"},    // Invalid glob pattern
+		{glob: "gcr.io/projectsigstore/*", input: "gcr.io/projectsigstore/cosign", match: true},
+		{glob: "gcr.io/projectsigstore/*", input: "us.gcr.io/projectsigstore/cosign", match: false},
+		{glob: "*gcr.io/projectsigstore/*", input: "gcr.io/projectsigstore/cosign", match: true},
+		{glob: "*gcr.io/projectsigstore/*", input: "gcr.io/projectsigstore2/cosign", match: false},
+		{glob: "*gcr.io/*/*", input: "us.gcr.io/projectsigstore/cosign", match: true}, // Does match with multiple '*'
+		{glob: "us.gcr.io/*/*", input: "us.gcr.io/projectsigstore/cosign", match: true},
+		{glob: "us.gcr.io/*/*", input: "gcr.io/projectsigstore/cosign", match: false},
+		{glob: "*.gcr.io/*/*", input: "asia.gcr.io/projectsigstore/cosign", match: true},
+		{glob: "*.gcr.io/*/*", input: "gcr.io/projectsigstore/cosign", match: false},
+		// Does not match since '*' only handles until next non-separator character '/'
+		// On Windows, '/' is not the separator and therefore it passes
+		{glob: "*gcr.io/*", input: "us.gcr.io/projectsigstore/cosign", match: false, windowsMatch: &struct{ match bool }{match: true}},
 	}
 	for _, tc := range tests {
-		got := GlobMatch(tc.input, tc.glob)
-		if got != tc.match {
-			t.Errorf("expected %v for glob: %q input: %q", tc.match, tc.glob, tc.input)
+		got, err := GlobMatch(tc.glob, tc.input)
+
+		if tc.errString != "" {
+			if tc.errString != err.Error() {
+				t.Errorf("expected %s for error: %s", tc.errString, err.Error())
+			}
+		} else if err != nil {
+			t.Errorf("unexpected error: %v for glob: %q input: %q", err, tc.glob, tc.input)
+		}
+
+		want := tc.match
+
+		// If OS is Windows, check if there is a different expected match value
+		if runtime.GOOS == "windows" && tc.windowsMatch != nil {
+			want = tc.windowsMatch.match
+		}
+
+		if got != want {
+			t.Errorf("expected %v for glob: %q input: %q", want, tc.glob, tc.input)
 		}
 	}
 }

pkg/apis/config/image_policies.go

@@ -93,7 +93,9 @@ func (p *ImagePolicyConfig) GetMatchingPolicies(image string) (map[string]webhoo
 	for k, v := range p.Policies {
 		for _, pattern := range v.Images {
 			if pattern.Glob != "" {
-				if GlobMatch(image, pattern.Glob) {
+				if matched, err := GlobMatch(pattern.Glob, image); err != nil {
+					lastError = err
+				} else if matched {
 					ret[k] = v
 				}
 			} else if pattern.Regex != "" {

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation.go

@@ -17,8 +17,8 @@ package v1alpha1
 import (
 	"context"
 	"fmt"
+	"path/filepath"
 	"regexp"
-	"strings"
 
 	"github.com/sigstore/cosign/pkg/apis/utils"
 	"knative.dev/pkg/apis"
@@ -202,20 +202,13 @@ func (identity *Identity) Validate(ctx context.Context) *apis.FieldError {
 	return errs
 }
 
-// ValidateGlob makes sure that if there's "*" specified it's the trailing
-// character.
+// ValidateGlob glob compilation by testing against empty string
 func ValidateGlob(glob string) *apis.FieldError {
-	c := strings.Count(glob, "*")
-	switch c {
-	case 0:
-		return nil
-	case 1:
-		if !strings.HasSuffix(glob, "*") {
-			return apis.ErrInvalidValue(glob, apis.CurrentField, "glob match supports only * as a trailing character")
-		}
-	default:
-		return apis.ErrInvalidValue(glob, apis.CurrentField, "glob match supports only a single * as a trailing character")
+	_, err := filepath.Match(glob, "")
+	if err != nil {
+		return apis.ErrInvalidValue(glob, apis.CurrentField, fmt.Sprintf("glob is invalid: %v", err))
 	}
+
 	return nil
 }
 

pkg/apis/cosigned/v1alpha1/clusterimagepolicy_validation_test.go

@@ -33,7 +33,7 @@ func TestImagePatternValidation(t *testing.T) {
 		{
 			name:        "Should fail when both regex and glob are present",
 			expectErr:   true,
-			errorString: "expected exactly one, got both: spec.images[0].glob, spec.images[0].regex\ninvalid value: **: spec.images[0].glob\nglob match supports only a single * as a trailing character\nmissing field(s): spec.authorities",
+			errorString: "expected exactly one, got both: spec.images[0].glob, spec.images[0].regex\nmissing field(s): spec.authorities",
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
 					Images: []ImagePattern{
@@ -58,28 +58,14 @@ func TestImagePatternValidation(t *testing.T) {
 			},
 		},
 		{
-			name:        "Glob should fail with multiple *",
+			name:        "Glob should fail with invalid glob",
 			expectErr:   true,
-			errorString: "invalid value: **: spec.images[0].glob\nglob match supports only a single * as a trailing character\nmissing field(s): spec.authorities",
+			errorString: "invalid value: [: spec.images[0].glob\nglob is invalid: syntax error in pattern\nmissing field(s): spec.authorities",
 			policy: ClusterImagePolicy{
 				Spec: ClusterImagePolicySpec{
 					Images: []ImagePattern{
 						{
-							Glob: "**",
-						},
-					},
-				},
-			},
-		},
-		{
-			name:        "Glob should fail with non-trailing *",
-			expectErr:   true,
-			errorString: "invalid value: foo*bar: spec.images[0].glob\nglob match supports only * as a trailing character\nmissing field(s): spec.authorities",
-			policy: ClusterImagePolicy{
-				Spec: ClusterImagePolicySpec{
-					Images: []ImagePattern{
-						{
-							Glob: "foo*bar",
+							Glob: "[",
 						},
 					},
 				},