fix(rootfs): only pivot_root when a mount namespace actually exists by Anand-240 · Pull Request #863 · urunc-dev/urunc

Anand-240 · 2026-07-30T11:33:19Z

Description

Exec() decides whether to pivot_root or chroot into the monitor's rootfs by checking whether findNS() returned an error for the mount namespace. findNS() returns a non-nil error in two different situations though: the namespace type is entirely absent from Linux.Namespaces, or it's present but not created yet (empty Path). The old code treated both the same way (withPivot := err != nil), so it ended up pivoting even when the mount namespace type was missing from the spec altogether.

When that happens, FormatNsenterInfo() never sets unix.CLONE_NEWNS, so no new mount namespace gets created. The process just stays in whatever mount namespace the caller (normally urunc create itself) is running in, which is usually the host's. pivot_root then runs against that namespace instead of an isolated one.

This mirrors the distinction joinSandboxNetNs already makes using ErrNotExistingNS: only pivot when findNS returns nil (joining an existing namespace) or ErrNotExistingNS (namespace entry present, about to be created). If the namespace type is missing from the spec entirely, fall back to chroot.

Also fixed the comment above this line, since it described the intended behavior but didn't match what the code actually did.

Related issues

Fixes pivot_root runs against the host's real root filesystem when the OCI spec omits a mount namespace, due to inverted logic in the pivot decision #861

How was this tested?

gofmt -l clean
Built and vetted with GOOS=linux GOARCH=amd64, no build/vet issues
Added TestFindNS in pkg/unikontainers/utils_test.go covering the three cases the fix depends on: namespace type absent from the spec, namespace present without a path yet, namespace present with an existing path
Ran the full unit test suite (go test ./pkg/unikontainers/... -v) in a golang container, not just the new test
CI unit tests pass on both amd64 and arm64

LLM usage

Used Claude to help investigate the bug (reported as #861) and put together this fix. Reviewed, tested, and pushed the follow-up fixes myself.

Checklist

I have read the contribution guide.
The linter passes locally (make lint).
The e2e tests of at least one tool pass locally (make test_ctr, make test_nerdctl, make test_docker, make test_crictl).
If LLMs were used: I have read the llm policy.

netlify · 2026-07-30T11:33:26Z

✅ Deploy Preview for urunc canceled.

Name	Link
🔨 Latest commit	`99c7be6`
🔍 Latest deploy log	https://app.netlify.com/projects/urunc/deploys/6a6b368167caea0008cb9305

netlify · 2026-07-30T11:33:27Z

✅ Deploy Preview for urunc canceled.

Name	Link
🔨 Latest commit	`f4bf52a`
🔍 Latest deploy log	https://app.netlify.com/projects/urunc/deploys/6a6b81db3db5d2000811395f

cmainas · 2026-07-30T12:01:41Z

Hello @Anand-240 ,

thank you for the PR, as mentioned in the contribution guide:

sign-off your commits
add yourself in https://github.com/urunc-dev/urunc/blob/main/.github/contributors.yaml

Also testing a PR against the tests that the same PR introduces is not a valid test. You should perform all the tests. This is a small change and it is ok, but as an author you are responsible to ensure your changes work and show that to us.

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 urunc-dev#861 Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com>

Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com>

cmainas

Hello @Anand-240 ,

thank you for the changes, there some things we need to fix for the unit test.

cmainas · 2026-07-30T13:44:00Z

+	t.Run("namespace type missing from spec", func(t *testing.T) {
+		t.Parallel()
+		namespaces := []specs.LinuxNamespace{
+			{Type: specs.NetworkNamespace, Path: "/proc/1/ns/net"},


We should use a dummy path here

Fixed, using /dummy/path now instead of a real proc path.

cmainas · 2026-07-30T13:44:28Z

+		assert.False(t, errors.Is(err, ErrNotExistingNS),
+			"a namespace type absent from the spec must not be reported as ErrNotExistingNS")


let's merge these lines

Merged into one line.

cmainas · 2026-07-30T13:46:45Z

+		nsPath := filepath.Join(t.TempDir(), "mnt")
+		require.NoError(t, os.WriteFile(nsPath, []byte{}, 0644))


Why do we need to create a file for this?

Good catch, didn't need it. t.TempDir() already gives a real existing directory so I just use that as the path directly now, no file needed.

cmainas · 2026-07-30T13:49:56Z

Please also include the LLM usage section in the PR description.

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 <anandprakashsrivastava68@gmail.com>

Anand-240 · 2026-07-30T17:03:08Z

Please also include the LLM usage section in the PR description.

Done, it's back in the description now.

cmainas

Thank you @Anand-240 .

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 PR: #863 Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

PR: #863 Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

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. PR: #863 Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

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 PR: #863 Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

PR: #863 Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

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. PR: #863 Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com> Reviewed-by: Charalampos Mainas <cmainas@nubificus.co.uk> Approved-by: Charalampos Mainas <cmainas@nubificus.co.uk>

Anand-240 force-pushed the fix/mount-namespace-pivot-decision branch from 99c7be6 to 18c0109 Compare July 30, 2026 11:37

Anand-240 added 2 commits July 30, 2026 17:58

chore: add myself to contributors list

2385fe8

Signed-off-by: Anand-240 <anandprakashsrivastava68@gmail.com>

Anand-240 force-pushed the fix/mount-namespace-pivot-decision branch from 18c0109 to 2385fe8 Compare July 30, 2026 12:31

cmainas added the ok-to-test label Jul 30, 2026

cmainas requested changes Jul 30, 2026

View reviewed changes

cmainas added the takeover label Jul 30, 2026

urunc-bot Bot changed the base branch from main to main-pr863 July 30, 2026 20:00

cmainas approved these changes Jul 30, 2026

View reviewed changes

cmainas merged commit b30a1e3 into urunc-dev:main-pr863 Jul 30, 2026
37 checks passed

		assert.False(t, errors.Is(err, ErrNotExistingNS),
		"a namespace type absent from the spec must not be reported as ErrNotExistingNS")

		nsPath := filepath.Join(t.TempDir(), "mnt")
		require.NoError(t, os.WriteFile(nsPath, []byte{}, 0644))

Uh oh!

Conversation

Anand-240 commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related issues

How was this tested?

LLM usage

Checklist

Uh oh!

netlify Bot commented Jul 30, 2026

✅ Deploy Preview for urunc canceled.

Uh oh!

netlify Bot commented Jul 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for urunc canceled.

Uh oh!

cmainas commented Jul 30, 2026

Uh oh!

cmainas left a comment

Choose a reason for hiding this comment

Uh oh!

cmainas Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Anand-240 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

cmainas Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Anand-240 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

cmainas Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

Anand-240 Jul 30, 2026

Choose a reason for hiding this comment

Uh oh!

cmainas commented Jul 30, 2026

Uh oh!

Anand-240 commented Jul 30, 2026

Uh oh!

cmainas left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Anand-240 commented Jul 30, 2026 •

edited

Loading

netlify Bot commented Jul 30, 2026 •

edited

Loading