Merge branch 'main' into let-go-for-unitialized-readonly-repo

pkg/archive/extractor.go

@@ -19,8 +19,10 @@ package archive
 import (
 	"archive/tar"
 	"compress/gzip"
+	"fmt"
 	"io"
 	"path/filepath"
+	"strings"
 
 	"github.com/sirupsen/logrus"
 
@@ -66,6 +68,16 @@ func (e *Extractor) writeFile(target string, tarRdr *tar.Reader) error {
 	return nil
 }
 
+// sanitizeArchivePath sanitizes archive file path from "G305: Zip Slip vulnerability"
+func sanitizeArchivePath(destDir, sourcePath string) (targetPath string, err error) {
+	targetPath = filepath.Join(destDir, sourcePath)
+	if strings.HasPrefix(targetPath, filepath.Clean(destDir)) {
+		return targetPath, nil
+	}
+
+	return "", fmt.Errorf("invalid archive path %q: escapes target directory", sourcePath)
+}
+
 func (e *Extractor) readBackup(tarRdr *tar.Reader) (string, error) {
 	dir, err := e.fs.TempDir("", "")
 	if err != nil {
@@ -84,7 +96,11 @@ func (e *Extractor) readBackup(tarRdr *tar.Reader) (string, error) {
 			return "", err
 		}
 
-		target := filepath.Join(dir, header.Name) //nolint:gosec // Internal usage. No need to check.
+		target, err := sanitizeArchivePath(dir, header.Name)
+		if err != nil {
+			e.log.Infof("error sanitizing archive path: %s", err.Error())
+			return "", err
+		}
 
 		switch header.Typeflag {
 		case tar.TypeDir:

pkg/archive/extractor_test.go

@@ -18,6 +18,7 @@ package archive
 
 import (
 	"archive/tar"
+	"bytes"
 	"compress/gzip"
 	"io"
 	"os"
@@ -87,6 +88,31 @@ func TestUnzipAndExtractBackup(t *testing.T) {
 	}
 }
 
+func TestUnzipAndExtractBackupRejectsPathTraversal(t *testing.T) {
+	ext := NewExtractor(test.NewLogger(), test.NewFakeFileSystem())
+
+	var buf bytes.Buffer
+	gzw := gzip.NewWriter(&buf)
+	tw := tar.NewWriter(gzw)
+
+	err := tw.WriteHeader(&tar.Header{
+		Name:     "../escape.txt",
+		Mode:     0600,
+		Typeflag: tar.TypeReg,
+		Size:     int64(len("data")),
+	})
+	require.NoError(t, err)
+
+	_, err = tw.Write([]byte("data"))
+	require.NoError(t, err)
+	require.NoError(t, tw.Close())
+	require.NoError(t, gzw.Close())
+
+	_, err = ext.UnzipAndExtractBackup(&buf)
+	require.Error(t, err)
+	require.Contains(t, err.Error(), "invalid archive path")
+}
+
 func createArchive(files []string, fs filesystem.Interface) (string, error) {
 	outName := "output.tar.gz"
 	out, err := fs.Create(outName)

pkg/cmd/cli/install/install.go

@@ -381,8 +381,8 @@ This is useful as a starting point for more customized installations.
 
   # velero install --provider azure --plugins velero/velero-plugin-for-microsoft-azure:v1.0.0 --bucket $BLOB_CONTAINER --secret-file ./credentials-velero --backup-location-config resourceGroup=$AZURE_BACKUP_RESOURCE_GROUP,storageAccount=$AZURE_STORAGE_ACCOUNT_ID[,subscriptionId=$AZURE_BACKUP_SUBSCRIPTION_ID] --snapshot-location-config apiTimeout=<YOUR_TIMEOUT>[,resourceGroup=$AZURE_BACKUP_RESOURCE_GROUP,subscriptionId=$AZURE_BACKUP_SUBSCRIPTION_ID]`,
 		Run: func(c *cobra.Command, args []string) {
-			cmd.CheckError(o.Validate(c, args, f))
 			cmd.CheckError(o.Complete(args, f))
+			cmd.CheckError(o.Validate(c, args, f))
 			cmd.CheckError(o.Run(c, f))
 		},
 	}

pkg/cmd/cli/install/install_test.go

@@ -17,11 +17,18 @@ limitations under the License.
 package install
 
 import (
+	"context"
 	"testing"
 
+	"github.com/spf13/cobra"
 	"github.com/spf13/pflag"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	corev1api "k8s.io/api/core/v1"
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+
+	factorymocks "github.com/vmware-tanzu/velero/pkg/client/mocks"
+	velerotest "github.com/vmware-tanzu/velero/pkg/test"
 )
 
 func TestPriorityClassNameFlag(t *testing.T) {
@@ -91,3 +98,168 @@ func TestPriorityClassNameFlag(t *testing.T) {
 		})
 	}
 }
+
+// makeValidateCmd returns a minimal *cobra.Command that satisfies output.ValidateFlags.
+func makeValidateCmd() *cobra.Command {
+	c := &cobra.Command{}
+	// output.ValidateFlags only inspects the "output" flag; add it so validation passes.
+	c.Flags().StringP("output", "o", "", "output format")
+	return c
+}
+
+// configMapInNamespace builds a ConfigMap with a single JSON data entry in the given namespace.
+func configMapInNamespace(namespace, name, jsonValue string) *corev1api.ConfigMap {
+	return &corev1api.ConfigMap{
+		ObjectMeta: metav1.ObjectMeta{
+			Namespace: namespace,
+			Name:      name,
+		},
+		Data: map[string]string{
+			"config": jsonValue,
+		},
+	}
+}
+
+// TestValidateConfigMapsUseFactoryNamespace verifies that Validate resolves the target
+// namespace correctly for all three ConfigMap flags.
+//
+// The fix (Option B) calls Complete before Validate in NewCommand so that o.Namespace is
+// populated from f.Namespace() before VerifyJSONConfigs runs. Tests mirror that order by
+// calling Complete before Validate.
+func TestValidateConfigMapsUseFactoryNamespace(t *testing.T) {
+	const targetNS = "tenant-b"
+	const defaultNS = "default"
+
+	// Shared options that satisfy every other validation gate:
+	//   - NoDefaultBackupLocation=true + UseVolumeSnapshots=false skips provider/bucket/plugins checks
+	//   - NoSecret=true satisfies the secret-file check
+	baseOptions := func() *Options {
+		o := NewInstallOptions()
+		o.NoDefaultBackupLocation = true
+		o.UseVolumeSnapshots = false
+		o.NoSecret = true
+		return o
+	}
+
+	tests := []struct {
+		name       string
+		setupOpts  func(o *Options, cmName string)
+		cmJSON     string
+		wantErrMsg string // substring expected in error; empty means success
+	}{
+		{
+			name: "NodeAgentConfigMap found in factory namespace",
+			setupOpts: func(o *Options, cmName string) {
+				o.NodeAgentConfigMap = cmName
+			},
+			cmJSON: `{}`,
+		},
+		{
+			name: "NodeAgentConfigMap not found when only in default namespace",
+			setupOpts: func(o *Options, cmName string) {
+				o.NodeAgentConfigMap = cmName
+			},
+			cmJSON:     `{}`,
+			wantErrMsg: "--node-agent-configmap specified ConfigMap",
+		},
+		{
+			name: "RepoMaintenanceJobConfigMap found in factory namespace",
+			setupOpts: func(o *Options, cmName string) {
+				o.RepoMaintenanceJobConfigMap = cmName
+			},
+			cmJSON: `{}`,
+		},
+		{
+			name: "RepoMaintenanceJobConfigMap not found when only in default namespace",
+			setupOpts: func(o *Options, cmName string) {
+				o.RepoMaintenanceJobConfigMap = cmName
+			},
+			cmJSON:     `{}`,
+			wantErrMsg: "--repo-maintenance-job-configmap specified ConfigMap",
+		},
+		{
+			name: "BackupRepoConfigMap found in factory namespace",
+			setupOpts: func(o *Options, cmName string) {
+				o.BackupRepoConfigMap = cmName
+			},
+			cmJSON: `{}`,
+		},
+		{
+			name: "BackupRepoConfigMap not found when only in default namespace",
+			setupOpts: func(o *Options, cmName string) {
+				o.BackupRepoConfigMap = cmName
+			},
+			cmJSON:     `{}`,
+			wantErrMsg: "--backup-repository-configmap specified ConfigMap",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			const cmName = "my-config"
+
+			// Decide where to place the ConfigMap:
+			// "not found" cases put it in "default", so the factory namespace lookup misses it.
+			cmNamespace := targetNS
+			if tc.wantErrMsg != "" {
+				cmNamespace = defaultNS
+			}
+
+			cm := configMapInNamespace(cmNamespace, cmName, tc.cmJSON)
+			kbClient := velerotest.NewFakeControllerRuntimeClient(t, cm)
+
+			f := &factorymocks.Factory{}
+			f.On("Namespace").Return(targetNS)
+			f.On("KubebuilderClient").Return(kbClient, nil)
+
+			o := baseOptions()
+			tc.setupOpts(o, cmName)
+
+			// Mirror the NewCommand call order: Complete populates o.Namespace before Validate runs.
+			require.NoError(t, o.Complete([]string{}, f))
+
+			c := makeValidateCmd()
+			c.SetContext(context.Background())
+
+			err := o.Validate(c, []string{}, f)
+
+			if tc.wantErrMsg == "" {
+				require.NoError(t, err)
+			} else {
+				require.Error(t, err)
+				assert.Contains(t, err.Error(), tc.wantErrMsg)
+			}
+		})
+	}
+}
+
+// TestNewCommandRunClosureOrder covers the Run closure in NewCommand (the lines that were
+// reordered by the fix: Complete → Validate → Run).
+//
+// The closure uses CheckError which calls os.Exit on any error, so the only safe path is one
+// where all three steps return nil. DryRun=true causes o.Run to return after PrintWithFormat
+// (which is a no-op when no --output flag is set) without touching any cluster clients.
+func TestNewCommandRunClosureOrder(t *testing.T) {
+	const targetNS = "tenant-b"
+	const cmName = "my-config"
+
+	cm := configMapInNamespace(targetNS, cmName, `{}`)
+	kbClient := velerotest.NewFakeControllerRuntimeClient(t, cm)
+
+	f := &factorymocks.Factory{}
+	f.On("Namespace").Return(targetNS)
+	f.On("KubebuilderClient").Return(kbClient, nil)
+
+	c := NewCommand(f)
+	c.SetArgs([]string{
+		"--no-default-backup-location",
+		"--use-volume-snapshots=false",
+		"--no-secret",
+		"--dry-run",
+		"--node-agent-configmap", cmName,
+	})
+
+	// Execute drives the full Run closure: Complete populates o.Namespace, Validate
+	// looks up the ConfigMap in targetNS (succeeds), Run returns early via DryRun.
+	require.NoError(t, c.Execute())
+}

pkg/podexec/pod_command_executor.go

@@ -20,6 +20,7 @@ import (
 	"bytes"
 	"context"
 	"net/url"
+	"slices"
 	"time"
 
 	"github.com/pkg/errors"
@@ -184,10 +185,22 @@ func (e *defaultPodCommandExecutor) ExecutePodCommand(log logrus.FieldLogger, it
 }
 
 func ensureContainerExists(pod *corev1api.Pod, container string) error {
-	for _, c := range pod.Spec.Containers {
-		if c.Name == container {
-			return nil
-		}
+	existsAsMainContainer := slices.ContainsFunc(pod.Spec.Containers, func(c corev1api.Container) bool {
+		return c.Name == container
+	})
+
+	if existsAsMainContainer {
+		return nil
+	}
+
+	existsAsSidecar := slices.ContainsFunc(pod.Spec.InitContainers, func(c corev1api.Container) bool {
+		return c.RestartPolicy != nil &&
+			*c.RestartPolicy == corev1api.ContainerRestartPolicyAlways &&
+			c.Name == container
+	})
+
+	if existsAsSidecar {
+		return nil
 	}
 
 	return errors.Errorf("no such container: %q", container)

pkg/podexec/pod_command_executor_test.go

@@ -35,6 +35,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/client-go/rest"
 	"k8s.io/client-go/tools/remotecommand"
+	"k8s.io/utils/ptr"
 
 	v1 "github.com/vmware-tanzu/velero/pkg/apis/velero/v1"
 	velerotest "github.com/vmware-tanzu/velero/pkg/test"
@@ -253,6 +254,15 @@ func TestEnsureContainerExists(t *testing.T) {
 					Name: "foo",
 				},
 			},
+			InitContainers: []corev1api.Container{
+				{
+					Name: "baz",
+				},
+				{
+					Name:          "qux",
+					RestartPolicy: ptr.To(corev1api.ContainerRestartPolicyAlways),
+				},
+			},
 		},
 	}
 
@@ -260,7 +270,13 @@ func TestEnsureContainerExists(t *testing.T) {
 	require.EqualError(t, err, `no such container: "bar"`)
 
 	err = ensureContainerExists(pod, "foo")
-	assert.NoError(t, err)
+	require.NoError(t, err)
+
+	err = ensureContainerExists(pod, "baz")
+	require.EqualError(t, err, `no such container: "baz"`)
+
+	err = ensureContainerExists(pod, "qux")
+	require.NoError(t, err)
 }
 
 func TestPodCompeted(t *testing.T) {