[release/v2.2.x] fix: confine tarball extract and bound glob expansion

api/v1beta1/artifactgenerator_types.go

@@ -166,7 +166,7 @@ type CopyOperation struct {
 	// The format is "@artifact/path", the alias "artifact"
 	// refers to the root path of the generated artifact. When pathPattern
 	// is set, the path may use capture placeholders such as "{app}".
-	// +kubebuilder:validation:Pattern="^@(artifact)/(.*)$"
+	// +kubebuilder:validation:Pattern="^@artifact/([^/]{0,1}|[^./][^/]|[.][^./]|[^/]{3,})(/([^/]{0,1}|[^./][^/]|[.][^./]|[^/]{3,}))*$"
 	// +kubebuilder:validation:MinLength=1
 	// +kubebuilder:validation:MaxLength=1024
 	// +required
@@ -178,6 +178,7 @@ type CopyOperation struct {
 	// prefix of 'From'. Patterns without a separator (e.g. "*.md") match
 	// the file name at any depth.
 	// +kubebuilder:validation:MaxItems=100
+	// +kubebuilder:validation:items:MaxLength=1024
 	// +optional
 	Exclude []string `json:"exclude,omitempty"`
 

config/crd/bases/source.extensions.fluxcd.io_artifactgenerators.yaml

@@ -78,6 +78,7 @@ spec:
                               prefix of 'From'. Patterns without a separator (e.g. "*.md") match
                               the file name at any depth.
                             items:
+                              maxLength: 1024
                               type: string
                             maxItems: 100
                             type: array
@@ -112,7 +113,7 @@ spec:
                               is set, the path may use capture placeholders such as "{app}".
                             maxLength: 1024
                             minLength: 1
-                            pattern: ^@(artifact)/(.*)$
+                            pattern: ^@artifact/([^/]{0,1}|[^./][^/]|[.][^./]|[^/]{3,})(/([^/]{0,1}|[^./][^/]|[.][^./]|[^/]{3,}))*$
                             type: string
                         required:
                         - from

docs/spec/v1beta1/artifactgenerators.md

@@ -319,10 +319,13 @@ Each copy operation specifies how to copy files from sources into the generated
   Any file matched by `from` that also matches an exclude pattern will be ignored.
   Patterns are matched against paths relative to the source alias root or to the
   non-glob prefix of `from`. Patterns without a separator (e.g. `*.md`) match the
-  file name at any depth.
+  file name at any depth. Each pattern is limited to 1024 characters.
 - `strategy` (optional): Defines how to handle files during copy operations:
   `Overwrite` (default), `Merge` (for YAML files), or `Extract` (for tarball archives).
 
+The `from` and `exclude` glob patterns may each contain at most 20 commas
+inside `{}` alternation groups; patterns over the limit are rejected.
+
 Copy operations use `cp`-like semantics:
 
 - Operations are executed in order; later operations can overwrite files from earlier ones

internal/builder/builder.go

@@ -22,6 +22,7 @@ import (
 	"io/fs"
 	"os"
 	"path/filepath"
+	"slices"
 	"strings"
 
 	"github.com/bmatcuk/doublestar/v4"
@@ -152,6 +153,10 @@ func applyCopyOperation(ctx context.Context,
 		return fmt.Errorf("invalid copy source '%s': %w", op.From, err)
 	}
 
+	if err := validateGlobPattern(srcPattern); err != nil {
+		return fmt.Errorf("invalid copy source '%s': %w", op.From, err)
+	}
+
 	destRelPath, err := parseCopyDestinationRelative(op.To)
 	if err != nil {
 		return fmt.Errorf("invalid copy destination '%s': %w", op.To, err)
@@ -163,6 +168,9 @@ func applyCopyOperation(ctx context.Context,
 	}
 
 	for _, pattern := range op.Exclude {
+		if err := validateGlobPattern(pattern); err != nil {
+			return fmt.Errorf("invalid exclude pattern '%s': %w", pattern, err)
+		}
 		if _, err := doublestar.Match(pattern, "."); err != nil {
 			return fmt.Errorf("invalid exclude pattern '%s'", pattern)
 		}
@@ -187,7 +195,7 @@ func applyCopyOperation(ctx context.Context,
 
 	if !isGlobPattern {
 		// Direct path reference - check what it actually is first (cp-like behavior)
-		return applySingleSourceCopy(ctx, op, srcRoot, srcPattern, stagingRoot, stagingDir, destRelPath, destEndsWithSlash)
+		return applySingleSourceCopy(ctx, op, srcRoot, srcPattern, stagingRoot, destRelPath, destEndsWithSlash)
 	}
 
 	matches, err := getGlobMatchingEntries(op, srcRoot, srcPattern)
@@ -233,7 +241,7 @@ func applyCopyOperation(ctx context.Context,
 				// Ignore files that are not tarball archives and directories
 				continue
 			}
-			if err := extractTarball(ctx, srcRoot, match, stagingDir, destRelPath); err != nil {
+			if err := extractTarball(ctx, srcRoot, match, stagingRoot, destRelPath); err != nil {
 				return fmt.Errorf("failed to extract tarball '%s' to '%s': %w", match, destRelPath, err)
 			}
 		} else {
@@ -256,7 +264,6 @@ func applySingleSourceCopy(ctx context.Context,
 	srcRoot *os.Root,
 	srcPath string,
 	stagingRoot *os.Root,
-	stagingDir string,
 	destPath string,
 	destEndsWithSlash bool) error {
 	// Clean the source path to handle trailing slashes
@@ -279,7 +286,7 @@ func applySingleSourceCopy(ctx context.Context,
 		return applySingleDirectoryCopy(ctx, op, srcRoot, srcPath, stagingRoot, destPath)
 	}
 
-	return applySingleFileCopy(ctx, op, srcRoot, srcPath, stagingRoot, stagingDir, destPath, destEndsWithSlash)
+	return applySingleFileCopy(ctx, op, srcRoot, srcPath, stagingRoot, destPath, destEndsWithSlash)
 }
 
 // applySingleFileCopy handles copying a single file using cp-like semantics:
@@ -290,7 +297,6 @@ func applySingleFileCopy(ctx context.Context,
 	srcRoot *os.Root,
 	srcPath string,
 	stagingRoot *os.Root,
-	stagingDir string,
 	destPath string,
 	destEndsWithSlash bool) error {
 	// Check if the file should be excluded
@@ -303,7 +309,7 @@ func applySingleFileCopy(ctx context.Context,
 		if !isTarball(srcPath) {
 			return fmt.Errorf("extract strategy requires tarball file (.tar.gz or .tgz), got '%s'", srcPath)
 		}
-		return extractTarball(ctx, srcRoot, srcPath, stagingDir, destPath)
+		return extractTarball(ctx, srcRoot, srcPath, stagingRoot, destPath)
 	}
 
 	var finalDestPath string
@@ -346,6 +352,41 @@ func containsGlobChars(path string) bool {
 	return strings.ContainsAny(path, "*?[]")
 }
 
+// maxGlobAlternationCommas bounds glob alternation expansion. doublestar
+// re-matches the whole pattern once per alternative and cannot be interrupted,
+// so cost is the product of the group widths; commas bound that product at 2^n.
+// 20 is ~780ms worst case, 22 is ~3s.
+const maxGlobAlternationCommas = 20
+
+// validateGlobPattern bounds a pattern's alternation expansion. Only commas
+// inside a '{}' group count; a backslash escapes the next byte. Must run before
+// any matcher, including doublestar.Match.
+func validateGlobPattern(pattern string) error {
+	var commas, depth int
+	for i := 0; i < len(pattern); i++ {
+		switch pattern[i] {
+		case '\\':
+			i++ // escaped byte is a literal
+		case '{':
+			depth++
+		case '}':
+			if depth > 0 {
+				depth--
+			}
+		case ',':
+			if depth > 0 {
+				commas++
+			}
+		}
+	}
+
+	if commas > maxGlobAlternationCommas {
+		return fmt.Errorf("pattern has %d alternatives in '{}' groups, at most %d are allowed",
+			commas, maxGlobAlternationCommas)
+	}
+	return nil
+}
+
 // calculateGlobDestination determines the correct destination path for a glob match
 // to match cp-like behavior for different glob patterns:
 // - dir/** patterns strip the directory prefix (like cp -r dir/** dest/)
@@ -407,7 +448,18 @@ func parseCopyDestinationRelative(to string) (string, error) {
 		return "", fmt.Errorf("destination must start with '@artifact/'")
 	}
 
-	return strings.TrimPrefix(to, "@artifact/"), nil
+	destPath := strings.TrimPrefix(to, "@artifact/")
+	if destPath == "" {
+		return ".", nil
+	}
+	if !filepath.IsLocal(destPath) {
+		return "", fmt.Errorf("destination path must stay within the artifact root")
+	}
+	if slices.Contains(strings.Split(filepath.ToSlash(destPath), "/"), "..") {
+		return "", fmt.Errorf("destination path must not contain '..'")
+	}
+
+	return destPath, nil
 }
 
 // copyFileWithRoots copies a file from srcRoot to stagingRoot os.Root,

internal/builder/builder_internal_test.go

@@ -0,0 +1,136 @@
+/*
+Copyright 2026 The Flux authors
+
+Licensed under the Apache License, Version 2.0 (the "License");
+you may not use this file except in compliance with the License.
+You may obtain a copy of the License at
+
+    http://www.apache.org/licenses/LICENSE-2.0
+
+Unless required by applicable law or agreed to in writing, software
+distributed under the License is distributed on an "AS IS" BASIS,
+WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+See the License for the specific language governing permissions and
+limitations under the License.
+*/
+
+// This is the only white-box test file in the package. It is reserved for
+// defensive guards that no black-box test can reach, because an earlier layer
+// rejects the input before the guard runs. Anything reachable through Build
+// belongs in builder_test.go instead.
+
+package builder
+
+import (
+	"context"
+	"os"
+	"path/filepath"
+	"testing"
+
+	. "github.com/onsi/gomega"
+)
+
+// TestRelativeToBase covers the guard rejecting paths that escape basePath.
+// Build never produces such a pair: excludeBasePath is always either the copy
+// source itself or the literal prefix of its glob, so every walked path is
+// underneath it. The guard only matters if a future caller passes an
+// unrelated base.
+func TestRelativeToBase(t *testing.T) {
+	tests := []struct {
+		name     string
+		basePath string
+		filePath string
+		want     string
+		wantOK   bool
+	}{
+		{
+			name:     "empty base returns the path unchanged",
+			basePath: "",
+			filePath: filepath.Join("a", "b"),
+			want:     filepath.Join("a", "b"),
+			wantOK:   true,
+		},
+		{
+			name:     "current directory base returns the path unchanged",
+			basePath: ".",
+			filePath: filepath.Join("a", "b"),
+			want:     filepath.Join("a", "b"),
+			wantOK:   true,
+		},
+		{
+			name:     "path under base is made relative",
+			basePath: "a",
+			filePath: filepath.Join("a", "b", "c"),
+			want:     filepath.Join("b", "c"),
+			wantOK:   true,
+		},
+		{
+			name:     "trailing slash on base is cleaned",
+			basePath: "a" + string(filepath.Separator),
+			filePath: filepath.Join("a", "b"),
+			want:     "b",
+			wantOK:   true,
+		},
+		{
+			name:     "path equal to base",
+			basePath: filepath.Join("a", "b"),
+			filePath: filepath.Join("a", "b"),
+			want:     ".",
+			wantOK:   true,
+		},
+		{
+			name:     "leading dots in a name are not treated as traversal",
+			basePath: "a",
+			filePath: filepath.Join("a", "..b"),
+			want:     "..b",
+			wantOK:   true,
+		},
+		{
+			name:     "sibling of base escapes",
+			basePath: filepath.Join("a", "b"),
+			filePath: filepath.Join("a", "c"),
+			wantOK:   false,
+		},
+		{
+			name:     "parent of base escapes",
+			basePath: filepath.Join("a", "b"),
+			filePath: "a",
+			wantOK:   false,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			g := NewWithT(t)
+			got, ok := relativeToBase(tt.basePath, tt.filePath)
+			g.Expect(ok).To(Equal(tt.wantOK))
+			if !tt.wantOK {
+				g.Expect(got).To(BeEmpty())
+				return
+			}
+			g.Expect(got).To(Equal(tt.want))
+		})
+	}
+}
+
+func TestExtractTarballConfinesDestination(t *testing.T) {
+	g := NewWithT(t)
+	tmpDir := t.TempDir()
+	sourceDir := filepath.Join(tmpDir, "source")
+	stagingDir := filepath.Join(tmpDir, "staging")
+	g.Expect(os.MkdirAll(sourceDir, 0o755)).To(Succeed())
+	g.Expect(os.MkdirAll(stagingDir, 0o755)).To(Succeed())
+	g.Expect(os.WriteFile(filepath.Join(sourceDir, "manifests.tgz"), []byte("invalid"), 0o644)).To(Succeed())
+
+	srcRoot, err := os.OpenRoot(sourceDir)
+	g.Expect(err).ToNot(HaveOccurred())
+	defer srcRoot.Close()
+	stagingRoot, err := os.OpenRoot(stagingDir)
+	g.Expect(err).ToNot(HaveOccurred())
+	defer stagingRoot.Close()
+
+	err = extractTarball(context.Background(), srcRoot, "manifests.tgz", stagingRoot, "../escape")
+	g.Expect(err).To(MatchError(ContainSubstring("failed to create destination directory")))
+	g.Expect(filepath.Join(tmpDir, "escape")).ToNot(BeADirectory())
+	g.Expect(filepath.Join(tmpDir, "escape", "config.yaml")).ToNot(BeAnExistingFile())
+}