From e9875fe9658602c5cce127adfec0d7f1027e8821 Mon Sep 17 00:00:00 2001 From: Anand-240 Date: Thu, 30 Jul 2026 17:58:47 +0530 Subject: [PATCH 1/3] fix(rootfs): only pivot_root when a mount namespace actually exists Exec() decided whether to pivot_root or chroot by checking if findNS() returned an error for the mount namespace. findNS() returns a non-nil error both when the namespace type is missing from the spec entirely and when it's present but not created yet, so the old check (withPivot := err != nil) ended up pivoting even when the spec had no mount namespace at all. In that case FormatNsenterInfo() never sets CLONE_NEWNS, so no new mount namespace gets created and the process stays in whatever mount namespace the caller (normally urunc create) is running in, usually the host's. pivot_root then runs against that namespace instead of an isolated one. Use the same ErrNotExistingNS check joinSandboxNetNs already relies on for the network namespace: only pivot when findNS returns nil (joining an existing namespace) or ErrNotExistingNS (entry present, about to be created). Fall back to chroot when the namespace type is missing from the spec entirely. Fixes #861 Signed-off-by: Anand-240 --- pkg/unikontainers/unikontainers.go | 10 ++++--- pkg/unikontainers/utils_test.go | 45 ++++++++++++++++++++++++++++++ 2 files changed, 51 insertions(+), 4 deletions(-) diff --git a/pkg/unikontainers/unikontainers.go b/pkg/unikontainers/unikontainers.go index bc43bcace..b218c4cea 100644 --- a/pkg/unikontainers/unikontainers.go +++ b/pkg/unikontainers/unikontainers.go @@ -698,10 +698,12 @@ func (u *Unikontainer) Exec(metrics m.Writer) error { // pivot _, err = findNS(u.Spec.Linux.Namespaces, specs.MountNamespace) - // We just want to check if a mount namespace was define din the list - // Therefore, if there was no error and the mount namespace was found - // we can pivot. - withPivot := err != nil + // Only pivot if a mount namespace entry is actually present in the + // spec, either to join (err is nil) or to create (err is + // ErrNotExistingNS). If the entry is missing entirely, no new mount + // namespace gets created and we have to chroot instead, otherwise + // pivot_root would run against the caller's own root filesystem. + withPivot := err == nil || errors.Is(err, ErrNotExistingNS) err = changeRoot(rootfsParams.MonRootfs, withPivot) if err != nil { return err diff --git a/pkg/unikontainers/utils_test.go b/pkg/unikontainers/utils_test.go index b779b982e..6a3036e37 100644 --- a/pkg/unikontainers/utils_test.go +++ b/pkg/unikontainers/utils_test.go @@ -16,6 +16,7 @@ package unikontainers import ( "encoding/json" + "errors" "os" "path/filepath" "strconv" @@ -23,6 +24,7 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestWritePidFile(t *testing.T) { @@ -240,6 +242,49 @@ func TestRemovePreservesOrder(t *testing.T) { assert.Equal(t, []string{"b", "c", "d"}, result) } +// TestFindNS checks that findNS lets callers tell apart a namespace type +// missing from the spec from one that's present but not created yet +// (empty path). Exec() relies on that distinction to decide when it's +// safe to pivot_root. +func TestFindNS(t *testing.T) { + t.Parallel() + + t.Run("namespace type missing from spec", func(t *testing.T) { + t.Parallel() + namespaces := []specs.LinuxNamespace{ + {Type: specs.NetworkNamespace, Path: "/proc/1/ns/net"}, + } + path, err := findNS(namespaces, specs.MountNamespace) + assert.Empty(t, path) + assert.Error(t, err) + assert.False(t, errors.Is(err, ErrNotExistingNS), + "a namespace type absent from the spec must not be reported as ErrNotExistingNS") + }) + + t.Run("namespace present without a path yet", func(t *testing.T) { + t.Parallel() + namespaces := []specs.LinuxNamespace{ + {Type: specs.MountNamespace, Path: ""}, + } + path, err := findNS(namespaces, specs.MountNamespace) + assert.Empty(t, path) + assert.ErrorIs(t, err, ErrNotExistingNS) + }) + + t.Run("namespace present with an existing path", func(t *testing.T) { + t.Parallel() + nsPath := filepath.Join(t.TempDir(), "mnt") + require.NoError(t, os.WriteFile(nsPath, []byte{}, 0644)) + + namespaces := []specs.LinuxNamespace{ + {Type: specs.MountNamespace, Path: nsPath}, + } + path, err := findNS(namespaces, specs.MountNamespace) + assert.NoError(t, err) + assert.Equal(t, nsPath, path) + }) +} + func TestLoadSpec(t *testing.T) { t.Run("load spec success", func(t *testing.T) { t.Parallel() From 2385fe861393e2af949747d9e62837e3a020843c Mon Sep 17 00:00:00 2001 From: Anand-240 Date: Thu, 30 Jul 2026 17:58:52 +0530 Subject: [PATCH 2/3] chore: add myself to contributors list Signed-off-by: Anand-240 --- .github/contributors.yaml | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/.github/contributors.yaml b/.github/contributors.yaml index 115a711ba..994146348 100644 --- a/.github/contributors.yaml +++ b/.github/contributors.yaml @@ -113,3 +113,7 @@ users: pocopepe: name: Viju Sanjai email: avijusanjai@gmail.com + Anand-240: + name: Anand Prakash Srivastava + email: anandprakashsrivastava68@gmail.com + From f4bf52afe28d81628d38dc607cbc63e5cffbe731 Mon Sep 17 00:00:00 2001 From: Anand-240 Date: Thu, 30 Jul 2026 22:24:35 +0530 Subject: [PATCH 3/3] fix(tests): address review comments on TestFindNS Use a dummy path instead of a real proc path in the "namespace type missing from spec" case, merge the assert.False call and its message onto one line, and drop the unnecessary file creation in the "existing path" case since t.TempDir() already returns a real directory. Also fixes the gosec G306 warning on the removed WriteFile call. Signed-off-by: Anand-240 --- pkg/unikontainers/utils_test.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/pkg/unikontainers/utils_test.go b/pkg/unikontainers/utils_test.go index 6a3036e37..2d23ea335 100644 --- a/pkg/unikontainers/utils_test.go +++ b/pkg/unikontainers/utils_test.go @@ -24,7 +24,6 @@ import ( "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) func TestWritePidFile(t *testing.T) { @@ -252,13 +251,12 @@ func TestFindNS(t *testing.T) { t.Run("namespace type missing from spec", func(t *testing.T) { t.Parallel() namespaces := []specs.LinuxNamespace{ - {Type: specs.NetworkNamespace, Path: "/proc/1/ns/net"}, + {Type: specs.NetworkNamespace, Path: "/dummy/path"}, } path, err := findNS(namespaces, specs.MountNamespace) assert.Empty(t, path) assert.Error(t, err) - assert.False(t, errors.Is(err, ErrNotExistingNS), - "a namespace type absent from the spec must not be reported as ErrNotExistingNS") + assert.False(t, errors.Is(err, ErrNotExistingNS), "a namespace type absent from the spec must not be reported as ErrNotExistingNS") }) t.Run("namespace present without a path yet", func(t *testing.T) { @@ -273,8 +271,7 @@ func TestFindNS(t *testing.T) { t.Run("namespace present with an existing path", func(t *testing.T) { t.Parallel() - nsPath := filepath.Join(t.TempDir(), "mnt") - require.NoError(t, os.WriteFile(nsPath, []byte{}, 0644)) + nsPath := t.TempDir() namespaces := []specs.LinuxNamespace{ {Type: specs.MountNamespace, Path: nsPath},