diff --git a/client/build_test.go b/client/build_test.go index f6c662d221aa..4079bee61cd4 100644 --- a/client/build_test.go +++ b/client/build_test.go @@ -7,6 +7,7 @@ import ( "io" "os" "path/filepath" + "runtime" "strconv" "strings" "syscall" @@ -1363,21 +1364,22 @@ func testClientGatewayContainerPlatformPATH(t *testing.T, sb integration.Sandbox Platform *pb.Platform Expected string }{{ - "default path", - nil, - utilsystem.DefaultPathEnvUnix, + Name: "default path", + Expected: utilsystem.DefaultPathEnvUnix, }, { - "linux path", - &pb.Platform{OS: "linux"}, - utilsystem.DefaultPathEnvUnix, + Name: "linux path", + Platform: &pb.Platform{OS: "linux"}, + Expected: utilsystem.DefaultPathEnvUnix, }, { - "windows path", - &pb.Platform{OS: "windows"}, - utilsystem.DefaultPathEnvWindows, + Name: "windows path", + Platform: &pb.Platform{OS: "windows"}, }} for _, tt := range tests { t.Run(tt.Name, func(t *testing.T) { + if tt.Platform != nil && tt.Platform.OS != runtime.GOOS { + t.Skipf("skip %s test on %s", tt.Platform.OS, runtime.GOOS) + } ctr, err := c.NewContainer(ctx, client.NewContainerRequest{ Mounts: []client.Mount{{ Dest: "/", diff --git a/client/llb/exec.go b/client/llb/exec.go index faba8bddc72b..a17b38007360 100644 --- a/client/llb/exec.go +++ b/client/llb/exec.go @@ -161,7 +161,9 @@ func (e *ExecOp) Marshal(ctx context.Context, c *Constraints) (digest.Digest, [] } else if e.constraints.Platform != nil { os = e.constraints.Platform.OS } - env = env.SetDefault("PATH", system.DefaultPathEnv(os)) + if v, ok := system.DefaultPathEnv(os); ok { + env = env.SetDefault("PATH", v) + } } else { addCap(&e.constraints, pb.CapExecMetaSetsDefaultPath) } diff --git a/exporter/containerimage/writer.go b/exporter/containerimage/writer.go index 4e3e678fd7e6..813eb940c947 100644 --- a/exporter/containerimage/writer.go +++ b/exporter/containerimage/writer.go @@ -674,7 +674,9 @@ func defaultImageConfig() ([]byte, error) { img.Variant = pl.Variant img.RootFS.Type = "layers" img.Config.WorkingDir = "/" - img.Config.Env = []string{"PATH=" + system.DefaultPathEnv(pl.OS)} + if env, ok := system.DefaultPathEnv(pl.OS); ok { + img.Config.Env = []string{"PATH=" + env} + } dt, err := json.Marshal(img) return dt, errors.Wrap(err, "failed to create empty image config") } diff --git a/frontend/dockerfile/dockerfile2llb/convert.go b/frontend/dockerfile/dockerfile2llb/convert.go index 43279e5126d2..f830e783b719 100644 --- a/frontend/dockerfile/dockerfile2llb/convert.go +++ b/frontend/dockerfile/dockerfile2llb/convert.go @@ -628,7 +628,9 @@ func toDispatchState(ctx context.Context, dt []byte, opt ConvertOpt) (*dispatchS if d.platform != nil { osName = d.platform.OS } - d.image.Config.Env = append(d.image.Config.Env, "PATH="+system.DefaultPathEnv(osName)) + if env, ok := system.DefaultPathEnv(osName); ok { + d.image.Config.Env = append(d.image.Config.Env, "PATH="+env) + } } // initialize base metadata from image conf diff --git a/frontend/dockerfile/dockerfile2llb/image.go b/frontend/dockerfile/dockerfile2llb/image.go index b6b589e77654..9b659d8715a0 100644 --- a/frontend/dockerfile/dockerfile2llb/image.go +++ b/frontend/dockerfile/dockerfile2llb/image.go @@ -34,6 +34,8 @@ func emptyImage(platform ocispecs.Platform) dockerspec.DockerOCIImage { img.Variant = platform.Variant img.RootFS.Type = "layers" img.Config.WorkingDir = "/" - img.Config.Env = []string{"PATH=" + system.DefaultPathEnv(platform.OS)} + if env, ok := system.DefaultPathEnv(platform.OS); ok { + img.Config.Env = []string{"PATH=" + env} + } return img } diff --git a/frontend/dockerfile/dockerfile_test.go b/frontend/dockerfile/dockerfile_test.go index 3530b7a19131..d6dbefc08a47 100644 --- a/frontend/dockerfile/dockerfile_test.go +++ b/frontend/dockerfile/dockerfile_test.go @@ -1655,7 +1655,7 @@ COPY Dockerfile . entrypoint []string env []string }{ - {p: "windows/amd64", entrypoint: []string{"cmd", "/S", "/C", "foo bar"}, env: []string{"PATH=c:\\Windows\\System32;c:\\Windows"}}, + {p: "windows/amd64", entrypoint: []string{"cmd", "/S", "/C", "foo bar"}}, {p: "linux/amd64", entrypoint: []string{"/bin/sh", "-c", "foo bar"}, env: []string{"PATH=/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"}}, } { t.Run(exp.p, func(t *testing.T) { diff --git a/frontend/gateway/container/container.go b/frontend/gateway/container/container.go index f34f67c1ca00..a0905f5d666c 100644 --- a/frontend/gateway/container/container.go +++ b/frontend/gateway/container/container.go @@ -327,7 +327,9 @@ func (gwCtr *gatewayContainer) Start(ctx context.Context, req client.StartReques if procInfo.Meta.Cwd == "" { procInfo.Meta.Cwd = "/" } - procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "PATH", utilsystem.DefaultPathEnv(gwCtr.platform.OS)) + if env, ok := utilsystem.DefaultPathEnv(gwCtr.platform.OS); ok { + procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "PATH", env) + } if req.Tty { procInfo.Meta.Env = addDefaultEnvvar(procInfo.Meta.Env, "TERM", "xterm") } diff --git a/solver/llbsolver/ops/exec.go b/solver/llbsolver/ops/exec.go index 058d911353e2..65883e030088 100644 --- a/solver/llbsolver/ops/exec.go +++ b/solver/llbsolver/ops/exec.go @@ -464,7 +464,9 @@ func (e *ExecOp) Exec(ctx context.Context, g session.Group, inputs []solver.Resu if e.platform != nil { currentOS = e.platform.OS } - meta.Env = addDefaultEnvvar(meta.Env, "PATH", utilsystem.DefaultPathEnv(currentOS)) + if env, ok := utilsystem.DefaultPathEnv(currentOS); ok { + meta.Env = addDefaultEnvvar(meta.Env, "PATH", env) + } secretEnv, err := e.loadSecretEnv(ctx, g) if err != nil { diff --git a/util/system/path.go b/util/system/path.go index cdfff68007f5..ecaa6113c502 100644 --- a/util/system/path.go +++ b/util/system/path.go @@ -13,16 +13,36 @@ import ( // ':' character . const DefaultPathEnvUnix = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin" -// DefaultPathEnvWindows is windows style list of directories to search for -// executables. Each directory is separated from the next by a colon -// ';' character . -const DefaultPathEnvWindows = "c:\\Windows\\System32;c:\\Windows" +// DefaultPathEnvWindows is deliberately empty on Windows and the default path +// must be set by the image. BuildKit has no context of what the default +// path should be. +// +// Deprecated: Windows images must not have a default PATH set. +const DefaultPathEnvWindows = "" -func DefaultPathEnv(os string) string { +// DefaultPathEnv returns the default value for the PATH environment-variable +// if available for the given operating-system. The second return variable +// indicates whether a default exists. +// +// On Linux/Unix, this returns [DefaultPathEnvUnix]. On Windows, no default +// exists, and PATH must not be set as its image-dependent, and must be +// configured through other means. +// +// More details about default PATH values for Windows images can be found +// in the following GitHub discussions: +// +// - [moby/moby#13833] +// - [moby/buildkit#3158] +// - [containerd/containerd#9118] +// +// [moby/moby#13833]: https://github.com/moby/moby/pull/13833 +// [moby/buildkit#3158]: https://github.com/moby/buildkit/pull/3158 +// [containerd/containerd#9118]: https://github.com/containerd/containerd/pull/9118 +func DefaultPathEnv(os string) (string, bool) { if os == "windows" { - return DefaultPathEnvWindows + return "", false } - return DefaultPathEnvUnix + return DefaultPathEnvUnix, true } // NormalizePath cleans the path based on the operating system the path is meant for.