Merge pull request #2105 from giuseppe/fix-off-by-one-max-size

userns: fix off-by-one userns max size detection

userns.go

@@ -89,7 +89,7 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 {
 		passwdFile = filepath.Join(containerMount, "etc/passwd")
 	}
 	if groupFile == "" {
-		groupFile = filepath.Join(groupFile, "etc/group")
+		groupFile = filepath.Join(containerMount, "etc/group")
 	}
 
 	size := 0
@@ -103,10 +103,10 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 {
 				continue
 			}
 			if u.Uid > size && u.Uid != nobodyUser {
-				size = u.Uid
+				size = u.Uid + 1
 			}
 			if u.Gid > size && u.Gid != nobodyUser {
-				size = u.Gid
+				size = u.Gid + 1
 			}
 		}
 	}
@@ -118,7 +118,7 @@ func parseMountedFiles(containerMount, passwdFile, groupFile string) uint32 {
 				continue
 			}
 			if g.Gid > size && g.Gid != nobodyUser {
-				size = g.Gid
+				size = g.Gid + 1
 			}
 		}
 	}

userns_test.go

@@ -1,6 +1,8 @@
 package storage
 
 import (
+	"os"
+	"path/filepath"
 	"reflect"
 	"testing"
 
@@ -170,3 +172,87 @@ func TestGetAutoUserNSMapping(t *testing.T) {
 		})
 	}
 }
+
+func TestParseMountedFiles(t *testing.T) {
+	tests := []struct {
+		name          string
+		passwdContent string
+		groupContent  string
+		expectedMax   uint32
+	}{
+		{
+			name: "basic case",
+			passwdContent: `
+root:x:0:0:root:/root:/bin/bash
+user1:x:1000:1000::/home/user1:/bin/bash
+nobody:x:65534:65534:nobody:/nonexistent:/usr/sbin/nologin`,
+			groupContent: `
+root:x:0:
+user1:x:1000:
+nogroup:x:65534:`,
+			expectedMax: 1001,
+		},
+		{
+			name: "only passwd",
+			passwdContent: `
+root:x:0:0:root:/root:/bin/bash
+user1:x:4001:4001::/home/user1:/bin/bash
+nobody:x:65534:65534:nobody:/nonexistent:/usr/sbin/nologin`,
+			groupContent: "",
+			expectedMax:  4002,
+		},
+		{
+			name:          "only groups",
+			passwdContent: "",
+			groupContent: `
+root:x:0:
+admin:x:3000:
+nobody:x:65534:`,
+			expectedMax: 3001,
+		},
+		{
+			name:          "empty files",
+			passwdContent: "",
+			groupContent:  "",
+			expectedMax:   0,
+		},
+		{
+			name:          "invalid passwd file",
+			passwdContent: "FOOBAR",
+			groupContent:  "",
+			expectedMax:   0,
+		},
+		{
+			name:          "invalid groups file",
+			passwdContent: "",
+			groupContent:  "FOOBAR",
+			expectedMax:   0,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			tmpDir, err := os.MkdirTemp("", "containermount")
+			if err != nil {
+				t.Fatalf("Failed to create temp dir: %v", err)
+			}
+			defer os.RemoveAll(tmpDir)
+
+			passwdFile := filepath.Join(tmpDir, "passwd")
+			if err := os.WriteFile(passwdFile, []byte(tt.passwdContent), 0o644); err != nil {
+				t.Fatalf("Failed to write passwd file: %v", err)
+			}
+
+			groupFile := filepath.Join(tmpDir, "group")
+			if err := os.WriteFile(groupFile, []byte(tt.groupContent), 0o644); err != nil {
+				t.Fatalf("Failed to write group file: %v", err)
+			}
+
+			result := parseMountedFiles(tmpDir, passwdFile, groupFile)
+
+			if result != tt.expectedMax {
+				t.Errorf("Expected max %d, but got %d", tt.expectedMax, result)
+			}
+		})
+	}
+}