Merge branch 'patch-security' into 'main'

Prevent CACHE_FALLBACK_KEY from bypassing protection on Windows

See merge request https://gitlab.com/gitlab-org/gitlab-runner/-/merge_requests/6440

Merged-by: Ashvin Sharma <ashsharma@gitlab.com>
Approved-by: Vishal Tak <vtak@gitlab.com>

Author: Ashvin Sharma

shells/abstract.go

@@ -289,7 +289,8 @@ func (b *AbstractShell) extractCacheOrFallbackCachesWrapper(
 	// the fallback key from CACHE_FALLBACK_KEY
 	blockProtectedFallback := func(key string) bool {
 		const blockedSuffix = "-protected"
-		allowed := !strings.HasSuffix(key, blockedSuffix)
+		trimmedKey := strings.TrimRight(key, ". ")
+		allowed := !strings.HasSuffix(trimmedKey, blockedSuffix)
 		if !allowed {
 			w.Warningf("CACHE_FALLBACK_KEY %q not allowed to end in %q", key, blockedSuffix)
 		}

shells/abstract_test.go

@@ -1554,10 +1554,9 @@ func TestAbstractShell_extractCacheWithDefaultFallbackKey(t *testing.T) {
 	const cacheEnvFile = "/some/path/to/runner-cache-env"
 
 	type expectations struct {
-		cacheKeys   []string
-		usesEnvFile []bool
-		warning     []any
-		notices     [][]any
+		cacheKeys []string
+		warning   []any
+		notices   [][]any
 	}
 	type hashMode uint8
 	const (
@@ -1631,8 +1630,7 @@ func TestAbstractShell_extractCacheWithDefaultFallbackKey(t *testing.T) {
 					notices:   [][]any{{`Skipping cache extraction due to %v`, fmt.Errorf("empty cache key")}},
 				},
 				withHashing: {
-					cacheKeys:   []string{"test-cache-key", ".."},
-					usesEnvFile: []bool{true, false},
+					cacheKeys: []string{"test-cache-key", ".."},
 				},
 			},
 		},
@@ -1646,9 +1644,23 @@ func TestAbstractShell_extractCacheWithDefaultFallbackKey(t *testing.T) {
 					warning:   []any{"CACHE_FALLBACK_KEY %q not allowed to end in %q", "main-protected", "-protected"},
 				},
 				withHashing: {
-					cacheKeys:   []string{"test-cache-key"},
-					warning:     []any{"CACHE_FALLBACK_KEY %q not allowed to end in %q", "main-protected", "-protected"},
-					usesEnvFile: []bool{true},
+					cacheKeys: []string{"test-cache-key"},
+					warning:   []any{"CACHE_FALLBACK_KEY %q not allowed to end in %q", "main-protected", "-protected"},
+				},
+			},
+		},
+		"using trailing dot suffix": {
+			cacheType:                "test",
+			cacheKey:                 "test-cache-key",
+			cacheFallbackKeyVarValue: "main-protected.",
+			expectations: map[hashMode]expectations{
+				withoutHashing: {
+					cacheKeys: []string{"test-cache-key"},
+					warning:   []any{"CACHE_FALLBACK_KEY %q not allowed to end in %q", "main-protected.", "-protected"},
+				},
+				withHashing: {
+					cacheKeys: []string{"test-cache-key"},
+					warning:   []any{"CACHE_FALLBACK_KEY %q not allowed to end in %q", "main-protected.", "-protected"},
 				},
 			},
 		},
@@ -1679,8 +1691,7 @@ func TestAbstractShell_extractCacheWithDefaultFallbackKey(t *testing.T) {
 					notices:   [][]any{{`Skipping cache extraction due to %v`, fmt.Errorf("empty cache key")}},
 				},
 				withHashing: {
-					cacheKeys:   []string{"some-job-name/some-ref-name", "."},
-					usesEnvFile: []bool{true, false},
+					cacheKeys: []string{"some-job-name/some-ref-name", "."},
 				},
 			},
 		},