From 9cc2f54fbd2c75e484fb28536962ac52ef40b33a Mon Sep 17 00:00:00 2001 From: KR Ravindra <42912207+KR-Ravindra@users.noreply.github.com> Date: Tue, 8 Sep 2026 20:53:49 +0000 Subject: [PATCH] dockerfile: reject --chown by name with --link at parse time When --link is used and the worker supports MergeOp, the COPY/ADD FileOp runs against llb.Scratch() and the result is merged on top of the stage. llb.WithUser() marshals a non-numeric --chown value as a by-name lookup against the FileOp base, which for scratch is input -1, so the solver later fails with "invalid user index: -1" and nothing points back to the offending --chown. Validate this in dispatchCopy when the link path is taken and return an error naming the flag and asking for numeric uid[:gid]. Numeric IDs and "root" keep working, and workers without MergeOp are unchanged because they fall back to a regular copy where names resolve against the stage filesystem. Document the restriction in the --chown section. Signed-off-by: KR Ravindra <42912207+KR-Ravindra@users.noreply.github.com> --- .../dockerfile/dockerfile2llb/convert_copy.go | 20 +++++++++-- .../dockerfile/dockerfile2llb/convert_test.go | 36 +++++++++++++++++++ frontend/dockerfile/docs/reference.md | 4 ++- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/frontend/dockerfile/dockerfile2llb/convert_copy.go b/frontend/dockerfile/dockerfile2llb/convert_copy.go index 9e3a0ff4f529..4293f81c3d73 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_copy.go +++ b/frontend/dockerfile/dockerfile2llb/convert_copy.go @@ -49,9 +49,15 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { return err } + // cfg.opt.llbCaps can be nil in unit tests + useLink := cfg.opt.llbCaps != nil && cfg.opt.llbCaps.Supports(pb.CapMergeOp) == nil && cfg.link && cfg.chmod == "" + var copyOpt []llb.CopyOption if cfg.chown != "" { + if useLink && chownByName(cfg.chown) { + return errors.Errorf("--chown=%s: user and group names can't be resolved when used with --link, use numeric uid[:gid] instead", cfg.chown) + } copyOpt = append(copyOpt, llb.WithUser(cfg.chown)) } @@ -330,8 +336,7 @@ func dispatchCopy(d *dispatchState, cfg copyConfig) error { fileOpt = append(fileOpt, llb.IgnoreCache) } - // cfg.opt.llbCaps can be nil in unit tests - if cfg.opt.llbCaps != nil && cfg.opt.llbCaps.Supports(pb.CapMergeOp) == nil && cfg.link && cfg.chmod == "" { + if useLink { pgID := identity.NewID() d.cmdIndex-- // prefixCommand increases it pgName := prefixCommand(d, name, d.prefixPlatform, &platform, env) @@ -369,6 +374,17 @@ func isGitSource(src string) bool { return false } +// chownByName reports whether chown refers to a user or group by name rather +// than by numeric ID. Names are resolved from /etc/passwd and /etc/group of the +// FileOp base, which is empty when copying into a linked layer. +func chownByName(chown string) bool { + co, ok := llb.WithUser(chown).(llb.ChownOpt) + if !ok { + return false + } + return (co.User != nil && co.User.Name != "") || (co.Group != nil && co.Group.Name != "") +} + func containsWildcards(name string) bool { for i := 0; i < len(name); i++ { switch name[i] { diff --git a/frontend/dockerfile/dockerfile2llb/convert_test.go b/frontend/dockerfile/dockerfile2llb/convert_test.go index 154786263839..47406d4425ef 100644 --- a/frontend/dockerfile/dockerfile2llb/convert_test.go +++ b/frontend/dockerfile/dockerfile2llb/convert_test.go @@ -468,3 +468,39 @@ func TestSourceStateFromSourceOpWrappedCopy(t *testing.T) { assert.Equal(t, sourceOp.Identifier, rewrittenSourceOp.Identifier) assert.Equal(t, sourceOp.Attrs, rewrittenSourceOp.Attrs) } + +func TestCopyLinkChownByName(t *testing.T) { + t.Parallel() + + caps := pb.Caps.CapSet(pb.Caps.All()) + + for _, tc := range []struct { + name string + chown string + mergeOp bool + err string + }{ + {name: "numeric", chown: "1000:1000", mergeOp: true}, + {name: "root", chown: "root:root", mergeOp: true}, + {name: "user name", chown: "foo", mergeOp: true, err: "--chown=foo"}, + {name: "user and group names", chown: "foo:bar", mergeOp: true, err: "--chown=foo:bar"}, + {name: "group name", chown: "1000:bar", mergeOp: true, err: "--chown=1000:bar"}, + {name: "user name without merge op", chown: "foo:bar"}, + } { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + df := "FROM scratch\nCOPY --link --chown=" + tc.chown + " a /b\n" + opt := ConvertOpt{} + if tc.mergeOp { + opt.LLBCaps = &caps + } + _, err := Dockerfile2LLB(appcontext.Context(), []byte(df), opt) + if tc.err == "" { + require.NoError(t, err) + return + } + require.ErrorContains(t, err, tc.err) + require.ErrorContains(t, err, "--link") + }) + } +} diff --git a/frontend/dockerfile/docs/reference.md b/frontend/dockerfile/docs/reference.md index 1b30a383c3b8..f70fe9c718fd 100644 --- a/frontend/dockerfile/docs/reference.md +++ b/frontend/dockerfile/docs/reference.md @@ -1917,7 +1917,9 @@ COPY --chown=myuser:mygroup --chmod=644 files* /somedir/ When using names instead of numeric IDs, BuildKit resolves them using `/etc/passwd` and `/etc/group` in the container's root filesystem. If these files are missing or don't contain the specified names, the build fails. -Numeric IDs don't require this lookup. +Numeric IDs don't require this lookup. When combined with +[`--link`](#copy---link), files are copied into an empty filesystem, so names +can't be resolved and `--chown` must use numeric IDs. The `--chown` flag is not supported when building Windows containers.