diff --git a/.github/contributors.yaml b/.github/contributors.yaml index 7f7307a3..288f2bd4 100644 --- a/.github/contributors.yaml +++ b/.github/contributors.yaml @@ -128,3 +128,6 @@ users: OdysseasKalaitsidis: name: Odysseas Kalaitsidis email: odysseaskalaitsides@gmail.com + srinivasr: + name: B Srinivas Reddy + email: sriniv4sreddy@gmail.com diff --git a/pkg/unikontainers/utils.go b/pkg/unikontainers/utils.go index 90b9500c..f81f1e77 100644 --- a/pkg/unikontainers/utils.go +++ b/pkg/unikontainers/utils.go @@ -18,6 +18,7 @@ import ( "bytes" "context" "encoding/json" + "errors" "fmt" "io" "os" @@ -209,16 +210,20 @@ func remove(s []string, i int) []string { } func checkValidNsPath(path string) error { - // only set to join this namespace if it exists - if _, err := os.Lstat(path); err != nil { - return ErrNotExistingNS - } // do not allow namespace path with comma as we use it to separate // the namespace paths if strings.ContainsRune(path, ',') { return fmt.Errorf("invalid namespace path %s", path) } + // only set to join this namespace if it exists + if _, err := os.Lstat(path); err != nil { + if errors.Is(err, os.ErrNotExist) { + return ErrNotExistingNS + } + return fmt.Errorf("failed to validate namespace path %s: %w", path, err) + } + return nil } diff --git a/pkg/unikontainers/utils_test.go b/pkg/unikontainers/utils_test.go index 2d23ea33..065c7ca7 100644 --- a/pkg/unikontainers/utils_test.go +++ b/pkg/unikontainers/utils_test.go @@ -17,6 +17,7 @@ package unikontainers import ( "encoding/json" "errors" + "io/fs" "os" "path/filepath" "strconv" @@ -340,3 +341,73 @@ func TestLoadSpec(t *testing.T) { assert.Contains(t, err.Error(), "failed to parse specification json", "Expected specific error message") }) } + +// TestCheckValidNsPath checks that checkValidNsPath distinguishes between +// a missing namespace path (returns ErrNotExistingNS) and other filesystem +// errors (permission denied, I/O errors, etc.) which are wrapped and returned. +func TestCheckValidNsPath(t *testing.T) { + t.Parallel() + + t.Run("missing path returns ErrNotExistingNS", func(t *testing.T) { + t.Parallel() + err := checkValidNsPath("/nonexistent/path/that/does/not/exist") + assert.ErrorIs(t, err, ErrNotExistingNS) + }) + + t.Run("empty path returns ErrNotExistingNS", func(t *testing.T) { + t.Parallel() + err := checkValidNsPath("") + assert.ErrorIs(t, err, ErrNotExistingNS) + }) + + t.Run("valid path returns nil", func(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + err := checkValidNsPath(tmpDir) + assert.NoError(t, err) + }) + + t.Run("path with comma returns error", func(t *testing.T) { + t.Parallel() + tmpDir := t.TempDir() + commaPath := filepath.Join(tmpDir, "path,with,comma") + // Create the path first so it exists + err := os.MkdirAll(commaPath, 0755) + assert.NoError(t, err) + + err = checkValidNsPath(commaPath) + assert.Error(t, err) + assert.Contains(t, err.Error(), "invalid namespace path") + assert.NotEqual(t, ErrNotExistingNS, err) + }) + + t.Run("permission denied returns wrapped error", func(t *testing.T) { + t.Parallel() + // Skip when running as root since root bypasses directory permissions + if os.Geteuid() == 0 { + t.Skip("skipping permission-denied test when running as root") + } + // Create a directory with no permissions and a file inside it + // This simulates a real permission-denied error from os.Lstat + tmpDir := t.TempDir() + restrictedDir := filepath.Join(tmpDir, "restricted") + err := os.Mkdir(restrictedDir, 0755) + assert.NoError(t, err) + + restrictedFile := filepath.Join(restrictedDir, "file") + err = os.WriteFile(restrictedFile, []byte("test"), 0600) + assert.NoError(t, err) + + // Now remove all permissions from the directory + err = os.Chmod(restrictedDir, 0000) + assert.NoError(t, err) + t.Cleanup(func() { _ = os.Chmod(restrictedDir, 0755) }) // cleanup + + err = checkValidNsPath(restrictedFile) + assert.Error(t, err) + assert.NotEqual(t, ErrNotExistingNS, err) + assert.Contains(t, err.Error(), "failed to validate namespace path") + // Verify the underlying error is a permission error + assert.True(t, errors.Is(err, os.ErrPermission) || errors.Is(err, fs.ErrPermission)) + }) +}