enhance!: match valid_volumes with bmatcuk/doublestar (#1214)

`valid_volumes` patterns now match with doublestar instead of gobwas/glob, which was compiled without separators so `*` crossed slashes. doublestar is already in the module graph, keeps `*` within one path segment and matches host paths with the OS separator, which fixes Windows patterns where a backslash was read as an escape.

Breaking: a host pattern that relied on `*` matching nested paths needs `**`, e.g. `/data/**` instead of `/data/*`. Named volumes and the bare `**` are unaffected.
Co-authored-by: bircni <bircni@icloud.com>
Reviewed-on: https://gitea.com/gitea/runner/pulls/1214
Reviewed-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <2021+silverwind@noreply.gitea.com>

act/container/docker_run.go

@@ -28,13 +28,13 @@ import (
 	"gitea.com/gitea/runner/act/filecollector"
 
 	"dario.cat/mergo"
+	"github.com/bmatcuk/doublestar/v4"
 	cerrdefs "github.com/containerd/errdefs"
 	"github.com/docker/cli/cli/compose/loader"
 	"github.com/docker/cli/cli/connhelper"
 	"github.com/go-git/go-billy/v5/helper/polyfill"
 	"github.com/go-git/go-billy/v5/osfs"
 	"github.com/go-git/go-git/v5/plumbing/format/gitignore"
-	"github.com/gobwas/glob"
 	"github.com/joho/godotenv"
 	"github.com/moby/moby/api/pkg/stdcopy"
 	"github.com/moby/moby/api/types/container"
@@ -1291,15 +1291,15 @@ func overlayVolumes(dst, src *container.HostConfig) {
 
 type validVolumeMatcher struct {
 	allowAll bool
-	named    []*glob.Pattern
-	host     []*glob.Pattern
+	named    []string
+	host     []string
 }
 
 func newValidVolumeMatcher(ctx context.Context, validVolumes []string) validVolumeMatcher {
 	logger := common.Logger(ctx)
 	ret := validVolumeMatcher{
-		named: make([]*glob.Pattern, 0, len(validVolumes)),
-		host:  make([]*glob.Pattern, 0, len(validVolumes)),
+		named: make([]string, 0, len(validVolumes)),
+		host:  make([]string, 0, len(validVolumes)),
 	}
 
 	for _, v := range validVolumes {
@@ -1308,10 +1308,10 @@ func newValidVolumeMatcher(ctx context.Context, validVolumes []string) validVolu
 			continue
 		}
 		if !isHostVolumePattern(v) {
-			if g, err := glob.Compile(v); err != nil {
-				logger.Errorf("create glob from %s error: %v", v, err)
+			if doublestar.ValidatePattern(v) {
+				ret.named = append(ret.named, v)
 			} else {
-				ret.named = append(ret.named, g)
+				logger.Errorf("invalid volume pattern %s", v)
 			}
 			continue
 		}
@@ -1320,10 +1320,10 @@ func newValidVolumeMatcher(ctx context.Context, validVolumes []string) validVolu
 			logger.Errorf("normalize volume pattern %s error: %v", v, err)
 			continue
 		}
-		if g, err := glob.Compile(normalized); err != nil {
-			logger.Errorf("create glob from %s error: %v", normalized, err)
+		if doublestar.ValidatePathPattern(normalized) {
+			ret.host = append(ret.host, normalized)
 		} else {
-			ret.host = append(ret.host, g)
+			logger.Errorf("invalid volume pattern %s", normalized)
 		}
 	}
 
@@ -1339,15 +1339,15 @@ func (m validVolumeMatcher) isValid(source string, sourceType mount.Type) bool {
 		if err != nil {
 			return false
 		}
-		for _, g := range m.host {
-			if g.Match(normalized) {
+		for _, pattern := range m.host {
+			if doublestar.PathMatchUnvalidated(pattern, normalized) {
 				return true
 			}
 		}
 		return false
 	}
-	for _, g := range m.named {
-		if g.Match(source) {
+	for _, pattern := range m.named {
+		if doublestar.MatchUnvalidated(pattern, source) {
 			return true
 		}
 	}

act/container/docker_run_test.go

@@ -560,11 +560,14 @@ func TestCheckVolumes(t *testing.T) {
 				"shared_volume",
 				"/home/test/data",
 				"/etc/conf.d/*.json",
+				"/opt/**",
 			},
 			binds: []string{
 				"shared_volume:/shared_volume",
 				"/home/test/data:/test_data",
 				"/etc/conf.d/base.json:/config/base.json",
+				"/etc/conf.d/sub/nested.json:/config/nested.json",
+				"/opt/app/cache:/cache",
 				"sql_data:/sql_data",
 				"/secrets/keys:/keys",
 			},
@@ -572,6 +575,7 @@ func TestCheckVolumes(t *testing.T) {
 				"shared_volume:/shared_volume",
 				"/home/test/data:/test_data",
 				"/etc/conf.d/base.json:/config/base.json",
+				"/opt/app/cache:/cache",
 			},
 		},
 	}

go.mod

@@ -9,6 +9,7 @@ require (
 	dario.cat/mergo v1.0.2
 	gitea.dev/actionslib v1.2.1
 	github.com/avast/retry-go/v5 v5.0.0
+	github.com/bmatcuk/doublestar/v4 v4.10.0
 	github.com/containerd/errdefs v1.0.0
 	github.com/creack/pty v1.1.24
 	github.com/distribution/reference v0.6.0
@@ -17,7 +18,6 @@ require (
 	github.com/docker/go-units v0.5.0
 	github.com/go-git/go-billy/v5 v5.9.1
 	github.com/go-git/go-git/v5 v5.19.2
-	github.com/gobwas/glob v1.0.0
 	github.com/google/go-cmp v0.7.0
 	github.com/joho/godotenv v1.5.1
 	github.com/julienschmidt/httprouter v1.3.0
@@ -58,7 +58,6 @@ require (
 	github.com/Microsoft/go-winio v0.6.2 // indirect
 	github.com/ProtonMail/go-crypto v1.4.1 // indirect
 	github.com/beorn7/perks v1.0.1 // indirect
-	github.com/bmatcuk/doublestar/v4 v4.10.0 // indirect
 	github.com/cespare/xxhash/v2 v2.3.0 // indirect
 	github.com/cloudflare/circl v1.6.5 // indirect
 	github.com/containerd/errdefs/pkg v0.3.0 // indirect

go.sum

@@ -75,8 +75,6 @@ github.com/go-logr/stdr v1.2.2 h1:hSWxHoqTgW2S2qGc0LTAI563KZ5YKYRhT3MFKZMbjag=
 github.com/go-logr/stdr v1.2.2/go.mod h1:mMo/vtBO5dYbehREoey6XUKy/eSumjCCveDpRre4VKE=
 github.com/go-viper/mapstructure/v2 v2.5.0 h1:vM5IJoUAy3d7zRSVtIwQgBj7BiWtMPfmPEgAXnvj1Ro=
 github.com/go-viper/mapstructure/v2 v2.5.0/go.mod h1:oJDH3BJKyqBA2TXFhDsKDGDTlndYOZ6rGS0BRZIxGhM=
-github.com/gobwas/glob v1.0.0 h1:p+FKbLEIsK1yZ39/OINwFvqNb5oyPY4H8xcy6uYu8dg=
-github.com/gobwas/glob v1.0.0/go.mod h1:oWCdo522i2P1n/hMXGNWs7yoV4wy/ciZuUIbvKj5rkc=
 github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8 h1:f+oWsMOmNPc8JmEHVZIycC7hBoQxHH9pNKQORJNozsQ=
 github.com/golang/groupcache v0.0.0-20241129210726-2c02b8208cf8/go.mod h1:wcDNUvekVysuuOpQKo3191zZyTpiI6se1N1ULghS0sw=
 github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8=

internal/pkg/config/config.example.yaml

@@ -232,7 +232,8 @@ container:
   # If it's empty, /workspace will be used.
   # Purely numeric subdirectories under this path are reserved for task workspaces and may be removed by idle cleanup.
   #workdir_parent:
-  # Volumes (including bind mounts) can be mounted to containers. Glob syntax is supported, see https://github.com/gobwas/glob
+  # Volumes (including bind mounts) can be mounted to containers. Glob syntax is supported, see https://github.com/bmatcuk/doublestar#patterns
+  # `*` matches within a single path segment, `**` also matches across path separators.
   # You can specify multiple volumes. If the sequence is empty, no volumes can be mounted.
   # For example, if you only allow containers to mount the `data` volume and all the json files in `/src`, you should change the config to:
   # valid_volumes: