Merge pull request #4954 from haytok/validate-mount-destination-not-root

fix: reject `/` as the `-v` destination in nerdctl run for Docker com…

Author: AkihiroSuda

cmd/nerdctl/container/container_run_mount_linux_test.go

@@ -32,6 +32,7 @@ import (
 	"github.com/containerd/nerdctl/mod/tigron/tig"
 
 	"github.com/containerd/nerdctl/v2/cmd/nerdctl/helpers"
+	"github.com/containerd/nerdctl/v2/pkg/mountutil"
 	"github.com/containerd/nerdctl/v2/pkg/rootlessutil"
 	"github.com/containerd/nerdctl/v2/pkg/testutil"
 	"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
@@ -762,3 +763,30 @@ func TestBindMountWhenHostFolderDoesNotExist(t *testing.T) {
 	_, err = os.Stat(hp)
 	assert.ErrorIs(t, err, os.ErrNotExist)
 }
+
+func TestRunVolumeWithRootDestination(t *testing.T) {
+	testCase := nerdtest.Setup()
+
+	testCase.Cleanup = func(data test.Data, helpers test.Helpers) {
+		helpers.Anyhow("rm", "-f", data.Identifier())
+	}
+
+	testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand {
+		return helpers.Command("run", "-d", "--name", data.Identifier(),
+			"-v", data.Temp().Dir()+":/", testutil.AlpineImage)
+	}
+
+	testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected {
+		return &test.Expected{
+			ExitCode: expect.ExitCodeGenericFail,
+			Errors:   []error{mountutil.ErrVolumeTargetIsRoot},
+			Output: func(stdout string, t tig.T) {
+				psOutput := helpers.Capture("ps", "-a", "--format", "{{.Names}}")
+				assert.Assert(t, !strings.Contains(psOutput, data.Identifier()),
+					"no container should be created when the volume destination is '/'")
+			},
+		}
+	}
+
+	testCase.Run(t)
+}

cmd/nerdctl/container/container_run_mount_windows_test.go

@@ -17,14 +17,21 @@
 package container
 
 import (
+	"errors"
 	"fmt"
 	"os"
+	"strings"
 	"testing"
 
 	"gotest.tools/v3/assert"
 
+	"github.com/containerd/nerdctl/mod/tigron/expect"
+	"github.com/containerd/nerdctl/mod/tigron/test"
+	"github.com/containerd/nerdctl/mod/tigron/tig"
+
 	"github.com/containerd/nerdctl/v2/pkg/inspecttypes/dockercompat"
 	"github.com/containerd/nerdctl/v2/pkg/testutil"
+	"github.com/containerd/nerdctl/v2/pkg/testutil/nerdtest"
 )
 
 func TestRunMountVolume(t *testing.T) {
@@ -213,3 +220,30 @@ func TestRunMountVolumeSpec(t *testing.T) {
 	// If -v is an empty string, it will be ignored
 	base.Cmd("run", "--rm", "-v", "", testutil.CommonImage).AssertOK()
 }
+
+func TestRunVolumeWithDriveRootDestination(t *testing.T) {
+	testCase := nerdtest.Setup()
+
+	testCase.Cleanup = func(data test.Data, helpers test.Helpers) {
+		helpers.Anyhow("rm", "-f", data.Identifier())
+	}
+
+	testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand {
+		return helpers.Command("run", "-d", "--name", data.Identifier(),
+			"-v", data.Temp().Dir()+`:C:\.`, testutil.CommonImage)
+	}
+
+	testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected {
+		return &test.Expected{
+			ExitCode: expect.ExitCodeGenericFail,
+			Errors:   []error{errors.New("destination path (c:\\\\) cannot be 'c:' or 'c:\\\\'")},
+			Output: func(stdout string, t tig.T) {
+				psOutput := helpers.Capture("ps", "-a", "--format", "{{.Names}}")
+				assert.Assert(t, !strings.Contains(psOutput, data.Identifier()),
+					"no container should be created when the volume destination is the drive root")
+			},
+		}
+	}
+
+	testCase.Run(t)
+}

pkg/mountutil/mountutil_linux_test.go

@@ -259,6 +259,10 @@ func TestProcessFlagV(t *testing.T) {
 			rawSpec: `/mnt/foo:./foo`,
 			err:     "expected an absolute path, got \"./foo\"",
 		},
+		{
+			rawSpec: `./:/`,
+			err:     "invalid specification: destination can't be '/'",
+		},
 	}
 
 	for _, tt := range tests {

pkg/mountutil/mountutil_unix.go

@@ -16,16 +16,26 @@
    limitations under the License.
 */
 
+/*
+   Portions from https://github.com/moby/moby/blob/docker-v29.5.2/daemon/volume/mounts/linux_parser.go
+   Copyright (C) Docker/Moby authors.
+   Licensed under the Apache License, Version 2.0
+   NOTICE: https://github.com/moby/moby/blob/docker-v29.5.2/NOTICE
+*/
+
 package mountutil
 
 import (
+	"errors"
 	"fmt"
 	"path/filepath"
 	"strings"
 
 	"github.com/containerd/nerdctl/v2/pkg/mountutil/volumestore"
 )
 
+var ErrVolumeTargetIsRoot = errors.New("invalid specification: destination can't be '/'")
+
 func splitVolumeSpec(s string) ([]string, error) {
 	s = strings.TrimLeft(s, ":")
 	split := strings.Split(s, ":")
@@ -48,7 +58,17 @@ func cleanMount(p string) string {
 	return filepath.Clean(p)
 }
 
+func validateNotRoot(p string) error {
+	if cleanMount(p) == "/" {
+		return ErrVolumeTargetIsRoot
+	}
+	return nil
+}
+
 func isValidPath(s string) (bool, error) {
+	if err := validateNotRoot(s); err != nil {
+		return false, err
+	}
 	if filepath.IsAbs(s) {
 		return true, nil
 	}

pkg/mountutil/mountutil_windows.go

@@ -161,7 +161,18 @@ func cleanMount(p string) string {
 	return filepath.Clean(p)
 }
 
+func validateNotRoot(p string) error {
+	p = strings.ToLower(cleanMount(p))
+	if p == "c:" || p == `c:\` {
+		return fmt.Errorf(`destination path (%v) cannot be 'c:' or 'c:\'`, p)
+	}
+	return nil
+}
+
 func isValidPath(s string) (bool, error) {
+	if err := validateNotRoot(s); err != nil {
+		return false, err
+	}
 	if isNamedPipe(s) || filepath.IsAbs(s) {
 		return true, nil
 	}

pkg/mountutil/mountutil_windows_test.go

@@ -262,6 +262,10 @@ func TestProcessFlagV(t *testing.T) {
 			rawSpec: `C:\TestVolume\Path:TestVolume`,
 			err:     "expected an absolute path or a named pipe, got \"TestVolume\"",
 		},
+		{
+			rawSpec: `C:\TestVolume\Path:c:\.`,
+			err:     "destination path (c:\\) cannot be 'c:' or 'c:\\'",
+		},
 	}
 
 	for _, tt := range tests {