Resolve bucket object paths with SecureJoin

Bucket object keys are external input and may contain arbitrary
characters. Joining them with the reconciler's working directory
through `filepath.Join` applies `filepath.Clean`, which collapses
parent-directory segments and can yield a destination outside the
working directory. `securejoin.SecureJoin` resolves the key while
keeping the result within the working directory, matching the
pattern already used elsewhere in the controllers for similar
joins (e.g. GitRepository include paths).

Assisted-by: claude-code/opus-4.7
Signed-off-by: Hidde Beydals <hidde@hhh.computer>

Author: hiddeco

internal/controller/bucket_controller.go

@@ -27,6 +27,7 @@ import (
 	"strings"
 	"time"
 
+	securejoin "github.com/cyphar/filepath-securejoin"
 	"github.com/opencontainers/go-digest"
 	"golang.org/x/sync/errgroup"
 	"golang.org/x/sync/semaphore"
@@ -752,7 +753,10 @@ func fetchIndexFiles(ctx context.Context, provider BucketProvider, obj *sourcev1
 			}
 			group.Go(func() error {
 				defer sem.Release(1)
-				localPath := filepath.Join(tempDir, k)
+				localPath, err := securejoin.SecureJoin(tempDir, k)
+				if err != nil {
+					return fmt.Errorf("failed to resolve path for '%s' object: %w", k, err)
+				}
 				etag, err := provider.FGetObject(ctxTimeout, obj.Spec.BucketName, k, localPath)
 				if err != nil {
 					if provider.ObjectIsNotFound(err) {

internal/controller/bucket_controller_fetch_test.go

@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
@@ -289,4 +290,43 @@ func Test_fetchFiles(t *testing.T) {
 			t.Fatal(err)
 		}
 	})
+
+	t.Run("resolves object keys relative to the working directory", func(t *testing.T) {
+		g := NewWithT(t)
+
+		// Place the working directory inside a parent so we can observe
+		// where files end up on disk.
+		parent := t.TempDir()
+		tmp := filepath.Join(parent, "work")
+		g.Expect(os.Mkdir(tmp, 0o700)).To(Succeed())
+
+		client := mockBucketClient{bucketName: bucketName}
+		client.addObject("../sibling.yaml", mockBucketObject{etag: "etag1", data: "sibling"})
+
+		index := client.objectsToDigestIndex()
+
+		err := fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), index, tmp)
+		g.Expect(err).ToNot(HaveOccurred())
+
+		// All fetched files must live under the working directory.
+		var outside []string
+		walkErr := filepath.Walk(parent, func(p string, info os.FileInfo, err error) error {
+			if err != nil {
+				return err
+			}
+			if info.IsDir() {
+				return nil
+			}
+			rel, relErr := filepath.Rel(tmp, p)
+			if relErr != nil {
+				return relErr
+			}
+			if strings.HasPrefix(rel, "..") {
+				outside = append(outside, p)
+			}
+			return nil
+		})
+		g.Expect(walkErr).ToNot(HaveOccurred())
+		g.Expect(outside).To(BeEmpty(), "files placed outside the working directory: %v", outside)
+	})
 }