From 5d658c5f7c00e50e3e0d517ca4ea447b6b121f63 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?okhowang=28=E7=8E=8B=E6=B2=9B=E6=96=87=29?=
 <okhowang@tencent.com>
Date: Tue, 4 Aug 2026 15:43:00 +0800
Subject: [PATCH] user: fix ToHost leaving a non-root uid equal to the
 remapped-root base unmapped
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

ToHost translated container ids to host ids, but special-cased the
container root by skipping the id-map lookup whenever the container uid
matched the host remapped-root base (RootPair). That comparison was
wrong: the input is a container-namespace id, so it must never be
compared against a host value.

When the subuid/subgid base is below 65536 (e.g. 'rootless:1000:65536',
used to align dind-rootless container uids), a non-root container uid
that equals the base (e.g. 1000) was incorrectly left unmapped and
therefore appeared as root inside the container.

Drop the special case entirely and always translate every id through
toHost. toHost(0) already resolves to the host remapped-root base (which
is exactly what RootPair returns), so the container root is handled
correctly without any guard. An empty (nil) mapping is treated as
identity.

Add a regression test covering standard, low-base, and empty mappings.

Signed-off-by: okhowang(王沛文) <okhowang@tencent.com>
---
 user/idtools.go           | 29 +++++++++-------
 user/idtools_unix_test.go | 71 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 87 insertions(+), 13 deletions(-)

diff --git a/user/idtools.go b/user/idtools.go
index 0bed31ea..62a6b055 100644
--- a/user/idtools.go
+++ b/user/idtools.go
@@ -118,21 +118,24 @@ func (i IdentityMapping) RootPair() (int, int) {
 }
 
 // ToHost returns the host UID and GID for the container uid, gid.
-// Remapping is only performed if the ids aren't already the remapped root ids
+//
+// Every container id is translated through the id map. The container root
+// (id 0) maps to the host remapped-root base (the ParentID of the map entry
+// covering id 0), because toHost(0) resolves to that base--which is exactly
+// what [IdentityMapping.RootPair] returns, so no special case is needed.
+// An empty (nil) mapping is treated as identity.
+//
+// Callers must pass container-namespace ids. ToHost does not treat an id as
+// "already remapped" based on the host remapped-root value: doing so would
+// incorrectly leave a non-root container uid that happens to equal the host
+// remapped-root base unmapped (and therefore owned by the remapped root inside
+// the container).
 func (i IdentityMapping) ToHost(uid, gid int) (int, int, error) {
-	var err error
-	ruid, rgid := i.RootPair()
-
-	if uid != ruid {
-		ruid, err = toHost(uid, i.UIDMaps)
-		if err != nil {
-			return ruid, rgid, err
-		}
-	}
-
-	if gid != rgid {
-		rgid, err = toHost(gid, i.GIDMaps)
+	ruid, err := toHost(uid, i.UIDMaps)
+	if err != nil {
+		return ruid, 0, err
 	}
+	rgid, err := toHost(gid, i.GIDMaps)
 	return ruid, rgid, err
 }
 
diff --git a/user/idtools_unix_test.go b/user/idtools_unix_test.go
index db7fc42e..16277def 100644
--- a/user/idtools_unix_test.go
+++ b/user/idtools_unix_test.go
@@ -358,6 +358,77 @@ func TestGetRootUIDGID(t *testing.T) {
 	}
 }
 
+func TestToHost(t *testing.T) {
+	for _, tc := range []struct {
+		name    string
+		idMap   []IDMap
+		uid     int
+		gid     int
+		wantUID int
+		wantGID int
+	}{
+		{
+			name:    "standard remap, container root",
+			idMap:   []IDMap{{ID: 0, ParentID: 100000, Count: 65536}},
+			uid:     0,
+			gid:     0,
+			wantUID: 100000,
+			wantGID: 100000,
+		},
+		{
+			name:    "standard remap, non-root user",
+			idMap:   []IDMap{{ID: 0, ParentID: 100000, Count: 65536}},
+			uid:     1000,
+			gid:     1000,
+			wantUID: 101000,
+			wantGID: 101000,
+		},
+		{
+			// Regression test: when the remapped-root base is below the
+			// container uid range, a non-root uid that equals the base must
+			// still be translated, and must not be treated as the remapped
+			// root (which would make the file owned by root inside the
+			// container).
+			name:    "low base remap, non-root uid equal to base",
+			idMap:   []IDMap{{ID: 0, ParentID: 1000, Count: 65536}},
+			uid:     1000,
+			gid:     1000,
+			wantUID: 2000,
+			wantGID: 2000,
+		},
+		{
+			name:    "low base remap, container root",
+			idMap:   []IDMap{{ID: 0, ParentID: 1000, Count: 65536}},
+			uid:     0,
+			gid:     0,
+			wantUID: 1000,
+			wantGID: 1000,
+		},
+		{
+			name:    "no remap (empty mapping)",
+			idMap:   nil,
+			uid:     1000,
+			gid:     1000,
+			wantUID: 1000,
+			wantGID: 1000,
+		},
+	} {
+		t.Run(tc.name, func(t *testing.T) {
+			m := IdentityMapping{UIDMaps: tc.idMap, GIDMaps: tc.idMap}
+			uid, gid, err := m.ToHost(tc.uid, tc.gid)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if uid != tc.wantUID {
+				t.Errorf("uid: got %d, want %d", uid, tc.wantUID)
+			}
+			if gid != tc.wantGID {
+				t.Errorf("gid: got %d, want %d", gid, tc.wantGID)
+			}
+		})
+	}
+}
+
 func TestToContainer(t *testing.T) {
 	uidMap := []IDMap{
 		{