From 40293bfdb897db4c8227bc1c356def74fdace6cd Mon Sep 17 00:00:00 2001 From: ump45nose Date: Sat, 31 Oct 2026 02:29:15 +0800 Subject: [PATCH] llb: parse chown IDs across the whole uid_t range WithUser() split a "user:group" string with parseUID(), which used strconv.ParseInt(.., 10, 32). Any value in 2^31..2^32-1 is a valid uid_t or gid_t, but ParseInt rejects it, so the numeric ID fell into the name branch and the daemon tried to resolve "2147483648" in /etc/passwd. When that lookup found nothing the copy silently stayed owned by root, so COPY --chown=2147483648 produced no error and no requested ownership. Parse the same way the daemon side already does in executor/oci: as an unsigned 32-bit value. A sign prefix is no longer accepted as an ID, which also removes the wrap-around that turned "-1" into 4294967295. Refs https://github.com/moby/buildkit/issues/7292 Signed-off-by: ump45nose --- client/llb/fileop.go | 5 ++++- client/llb/fileop_test.go | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) diff --git a/client/llb/fileop.go b/client/llb/fileop.go index 928d8503b..ddaf5b7b8 100644 --- a/client/llb/fileop.go +++ b/client/llb/fileop.go @@ -262,7 +262,10 @@ func parseUID(str string) (int, error) { if str == "root" { return 0, nil } - uid, err := strconv.ParseInt(str, 10, 32) + // uid_t/gid_t span 0..2^32-1, matching the daemon-side parser in + // executor/oci. ParseInt here would reject the upper half of that range and + // silently turn a numeric ID into a name lookup. + uid, err := strconv.ParseUint(str, 10, 32) if err != nil { return 0, err } diff --git a/client/llb/fileop_test.go b/client/llb/fileop_test.go index 196c875dc..3fd7afcf9 100644 --- a/client/llb/fileop_test.go +++ b/client/llb/fileop_test.go @@ -669,6 +669,43 @@ func TestFileOwnerWithUIDAndGID(t *testing.T) { require.Equal(t, 1001, int(mkdir.Owner.Group.User.(*pb.UserOpt_ByID).ByID)) } +// uid_t and gid_t span 0..2^32-1, so the upper half of that range must still be +// treated as a numeric ID and not fall back to a name lookup. +func TestFileOwnerWithUIDAndGIDAboveInt32(t *testing.T) { + t.Parallel() + + st := Image("foo").File(Mkdir("bar/baz", 0701, WithUser("2147483648:4294967295"))) + def, err := st.Marshal(t.Context()) + + require.NoError(t, err) + + _, arr := parseDef(t, def.Def) + + action := arr[1].Op.(*pb.Op_File).File.Actions[0] + mkdir := action.Action.(*pb.FileAction_Mkdir).Mkdir + + require.Equal(t, 2147483648, int(mkdir.Owner.User.User.(*pb.UserOpt_ByID).ByID)) + require.Equal(t, 4294967295, int(mkdir.Owner.Group.User.(*pb.UserOpt_ByID).ByID)) +} + +// A sign prefix is not a valid ID, so it stays a name lookup instead of +// wrapping around to the top of the ID range. +func TestFileOwnerWithNegativeUID(t *testing.T) { + t.Parallel() + + st := Image("foo").File(Mkdir("bar/baz", 0701, WithUser("-1"))) + def, err := st.Marshal(t.Context()) + + require.NoError(t, err) + + _, arr := parseDef(t, def.Def) + + action := arr[1].Op.(*pb.Op_File).File.Actions[0] + mkdir := action.Action.(*pb.FileAction_Mkdir).Mkdir + + require.Equal(t, "-1", mkdir.Owner.User.User.(*pb.UserOpt_ByName).ByName.Name) +} + func TestFileCopyOwner(t *testing.T) { t.Parallel()