diff --git a/client/client_test.go b/client/client_test.go index d3e66f24f9ae..3b16893cb0a2 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -349,6 +349,7 @@ func testIntegration(t *testing.T, funcs ...func(t *testing.T, sb integration.Sa integration.Run(t, integration.TestFuncs( // policy_test.go testProxyNetworkNoRootless, + testProxyNetworkGatewayExecEnvNoRootless, testProxyNetworkModesNoRootless, testProxyNetworkDefaultEgressNoRootless, ), diff --git a/client/policy_test.go b/client/policy_test.go index b53cf8ff32a7..42bb1807dd12 100644 --- a/client/policy_test.go +++ b/client/policy_test.go @@ -1,6 +1,7 @@ package client import ( + "bytes" "context" "crypto" "crypto/sha256" @@ -34,6 +35,7 @@ import ( sourcepolicypb "github.com/moby/buildkit/sourcepolicy/pb" "github.com/moby/buildkit/sourcepolicy/policysession" "github.com/moby/buildkit/util/entitlements" + "github.com/moby/buildkit/util/iohelper" "github.com/moby/buildkit/util/pgpsign" "github.com/moby/buildkit/util/testutil/integration" "github.com/moby/buildkit/util/testutil/workers" @@ -253,6 +255,85 @@ func testProxyNetworkNoRootless(t *testing.T, sb integration.Sandbox) { require.Equal(t, "unsuccessful_response", materialsErr.Incomplete[0].Reason) } +func testProxyNetworkGatewayExecEnvNoRootless(t *testing.T, sb integration.Sandbox) { + integration.SkipOnPlatform(t, "windows") + + ctx := sb.Context() + c, err := New(ctx, sb.Address()) + require.NoError(t, err) + defer c.Close() + childEnv := bytes.NewBuffer(nil) + + _, err = c.Build(ctx, SolveOpt{ProxyNetwork: true}, "proxy-network-gateway-exec-env", func(ctx context.Context, c gateway.Client) (*gateway.Result, error) { + def, err := llb.Image("busybox:latest").Marshal(ctx) + if err != nil { + return nil, err + } + res, err := c.Solve(ctx, gateway.SolveRequest{Definition: def.ToPB()}) + if err != nil { + return nil, err + } + ctr, err := c.NewContainer(ctx, gateway.NewContainerRequest{ + Mounts: []gateway.Mount{{ + Dest: "/", + MountType: opspb.MountType_BIND, + Ref: res.Ref, + }}, + }) + if err != nil { + return nil, err + } + pid1, err := ctr.Start(ctx, gateway.StartRequest{ + Args: []string{"sleep", "30"}, + Env: []string{ + "INIT_ONLY=must-not-leak", + "ALL_PROXY=http://initial-process-proxy.invalid", + }, + }) + if err != nil { + _ = ctr.Release(context.WithoutCancel(ctx)) + return nil, err + } + defer func() { + _ = ctr.Release(context.WithoutCancel(ctx)) + _ = pid1.Wait() + }() + + pid2, err := ctr.Start(ctx, gateway.StartRequest{ + Args: []string{"env"}, + Env: []string{ + "CHILD_ENV=preserved", + "ALL_PROXY=http://child-process-proxy.invalid", + }, + Stdout: &iohelper.NopWriteCloser{Writer: childEnv}, + }) + if err != nil { + return nil, err + } + if err := pid2.Wait(); err != nil { + return nil, err + } + return &gateway.Result{}, nil + }, nil) + require.NoError(t, err) + + env := strings.Split(strings.TrimSpace(childEnv.String()), "\n") + require.Contains(t, env, "CHILD_ENV=preserved") + require.Contains(t, env, "ALL_PROXY=http://child-process-proxy.invalid") + require.NotContains(t, env, "ALL_PROXY=http://initial-process-proxy.invalid") + require.NotContains(t, env, "INIT_ONLY=must-not-leak") + for _, name := range []string{"HTTP_PROXY", "HTTPS_PROXY", "http_proxy", "https_proxy", "NO_PROXY", "no_proxy"} { + found := false + for _, entry := range env { + if strings.HasPrefix(entry, name+"=") && len(entry) > len(name)+1 { + found = true + break + } + } + require.Truef(t, found, "%s is not set in the gateway exec environment:\n%s", name, childEnv.String()) + } +} + func testProxyNetworkModesNoRootless(t *testing.T, sb integration.Sandbox) { integration.SkipOnPlatform(t, "windows") workers.CheckFeatureCompat(t, sb, workers.FeatureCNINetwork) diff --git a/executor/containerdexecutor/executor.go b/executor/containerdexecutor/executor.go index d87a31126242..302098882e20 100644 --- a/executor/containerdexecutor/executor.go +++ b/executor/containerdexecutor/executor.go @@ -192,7 +192,7 @@ func (w *containerdExecutor) Run(ctx context.Context, id string, root executor.M } defer namespace.Close() if proxyNS, ok := namespace.(network.ProxyNamespace); ok { - meta.Env = append(meta.Env, proxyNS.ProxyEnv()...) + meta.Env = executor.ReplaceEnv(meta.Env, proxyNS.ProxyEnv()) cleanProxyCA, err := executor.InjectProxyCA(details.rootfsPath, proxyNS.ProxyCACert()) if err != nil { return nil, err @@ -315,6 +315,10 @@ func (w *containerdExecutor) Exec(ctx context.Context, id string, process execut } proc := spec.Process + if meta.Proxy != nil && len(meta.Env) > 0 { + meta.Env = executor.ReplaceEnv(meta.Env, network.FilterProxyEnv(proc.Env)) + process.Meta = meta + } if meta.User != "" { userSpec, err := getUserSpec(meta.User, details.rootfsPath) if err != nil { @@ -332,8 +336,8 @@ func (w *containerdExecutor) Exec(ctx context.Context, id string, process execut if meta.Cwd != "" { spec.Process.Cwd = meta.Cwd } - if len(process.Meta.Env) > 0 { - spec.Process.Env = process.Meta.Env + if len(meta.Env) > 0 { + proc.Env = meta.Env } fixProcessOutput(&process) diff --git a/executor/env_test.go b/executor/env_test.go new file mode 100644 index 000000000000..45c6e6f51ed1 --- /dev/null +++ b/executor/env_test.go @@ -0,0 +1,30 @@ +package executor + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestReplaceEnv(t *testing.T) { + env := []string{ + "FOO=one", + "HTTP_PROXY=http://upstream.example", + "http_proxy=http://upstream.example", + "NO_PROXY=example.com", + "BAR=two", + } + replacement := []string{ + "HTTP_PROXY=http://buildkit-proxy", + "http_proxy=http://buildkit-proxy", + "NO_PROXY=localhost", + } + + require.Equal(t, []string{ + "FOO=one", + "BAR=two", + "HTTP_PROXY=http://buildkit-proxy", + "http_proxy=http://buildkit-proxy", + "NO_PROXY=localhost", + }, ReplaceEnv(env, replacement)) +} diff --git a/executor/executor.go b/executor/executor.go index 999e248396a7..ef040ac1d119 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -4,6 +4,7 @@ import ( "context" "io" "net" + "strings" "syscall" "github.com/containerd/containerd/v2/core/mount" @@ -34,6 +35,24 @@ type Meta struct { RemoveMountStubsRecursive bool } +// ReplaceEnv removes entries whose names are present in replacement, then +// appends replacement in order. +func ReplaceEnv(env, replacement []string) []string { + names := make(map[string]struct{}, len(replacement)) + for _, entry := range replacement { + name, _, _ := strings.Cut(entry, "=") + names[name] = struct{}{} + } + out := make([]string, 0, len(env)+len(replacement)) + for _, entry := range env { + name, _, _ := strings.Cut(entry, "=") + if _, ok := names[name]; !ok { + out = append(out, entry) + } + } + return append(out, replacement...) +} + type MountableRef interface { Mount() ([]mount.Mount, func() error, error) IdentityMapping() *user.IdentityMapping diff --git a/executor/runcexecutor/executor.go b/executor/runcexecutor/executor.go index a301fecb294e..0838c5126f68 100644 --- a/executor/runcexecutor/executor.go +++ b/executor/runcexecutor/executor.go @@ -213,7 +213,7 @@ func (w *runcExecutor) Run(ctx context.Context, id string, root executor.Mount, return nil, err } if proxyNS, ok := namespace.(network.ProxyNamespace); ok { - meta.Env = append(meta.Env, proxyNS.ProxyEnv()...) + meta.Env = executor.ReplaceEnv(meta.Env, proxyNS.ProxyEnv()) } doReleaseNetwork := true defer func() { @@ -436,6 +436,7 @@ func exitError(ctx context.Context, cgroupPath string, err error, validExitCodes } func (w *runcExecutor) Exec(ctx context.Context, id string, process executor.ProcessInfo) (err error) { + meta := process.Meta // first verify the container is running, if we get an error assume the container // is in the process of being created and check again every 100ms or until // context is canceled. @@ -479,9 +480,13 @@ func (w *runcExecutor) Exec(ctx context.Context, id string, process executor.Pro if _, err := dec.Token(); !errors.Is(err, io.EOF) { return errors.Errorf("unexpected data after JSON spec object") } + if meta.Proxy != nil && len(meta.Env) > 0 { + meta.Env = executor.ReplaceEnv(meta.Env, network.FilterProxyEnv(spec.Process.Env)) + process.Meta = meta + } - if process.Meta.User != "" { - uid, gid, sgids, err := oci.GetUser(state.Rootfs, process.Meta.User) + if meta.User != "" { + uid, gid, sgids, err := oci.GetUser(state.Rootfs, meta.User) if err != nil { return err } @@ -492,14 +497,14 @@ func (w *runcExecutor) Exec(ctx context.Context, id string, process executor.Pro } } - spec.Process.Terminal = process.Meta.Tty - spec.Process.Args = process.Meta.Args - if process.Meta.Cwd != "" { - spec.Process.Cwd = process.Meta.Cwd + spec.Process.Terminal = meta.Tty + spec.Process.Args = meta.Args + if meta.Cwd != "" { + spec.Process.Cwd = meta.Cwd } - if len(process.Meta.Env) > 0 { - spec.Process.Env = process.Meta.Env + if len(meta.Env) > 0 { + spec.Process.Env = meta.Env } err = w.exec(ctx, id, spec.Process, process, nil) diff --git a/util/network/proxy.go b/util/network/proxy.go index b79aa922a7a1..f2fbdb038441 100644 --- a/util/network/proxy.go +++ b/util/network/proxy.go @@ -4,6 +4,7 @@ import ( "context" "io" "slices" + "strings" "sync" "github.com/moby/buildkit/solver/pb" @@ -34,6 +35,51 @@ type ProxyNamespace interface { ProxyCACert() []byte } +var proxyEnvNames = [...]struct { + name string + noProxy bool +}{ + {name: "HTTP_PROXY"}, + {name: "HTTPS_PROXY"}, + {name: "http_proxy"}, + {name: "https_proxy"}, + {name: "NO_PROXY", noProxy: true}, + {name: "no_proxy", noProxy: true}, +} + +// ProxyEnv returns the environment entries used to configure a process to use +// a BuildKit-owned HTTP(S) proxy. +func ProxyEnv(proxy, noProxy string) []string { + out := make([]string, 0, len(proxyEnvNames)) + for _, env := range proxyEnvNames { + value := proxy + if env.noProxy { + value = noProxy + } + out = append(out, env.name+"="+value) + } + return out +} + +// FilterProxyEnv returns entries whose names are emitted by ProxyEnv, preserving +// their original order. +func FilterProxyEnv(env []string) []string { + out := make([]string, 0, len(proxyEnvNames)) + for _, entry := range env { + name, _, ok := strings.Cut(entry, "=") + if !ok { + continue + } + for _, proxyEnv := range proxyEnvNames { + if name == proxyEnv.name { + out = append(out, entry) + break + } + } + } + return out +} + type ProxyMaterial struct { URL string Digest digest.Digest diff --git a/util/network/proxy_test.go b/util/network/proxy_test.go new file mode 100644 index 000000000000..cfb2d2d44798 --- /dev/null +++ b/util/network/proxy_test.go @@ -0,0 +1,21 @@ +package network + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestFilterProxyEnv(t *testing.T) { + require.Equal(t, []string{ + "HTTP_PROXY=http://buildkit-proxy", + "NO_PROXY=localhost", + }, FilterProxyEnv([]string{ + "PATH=/usr/bin", + "HTTP_PROXY=http://buildkit-proxy", + "FTP_PROXY=http://ftp-proxy", + "ALL_PROXY=http://initial-process-proxy", + "all_proxy=http://initial-process-proxy", + "NO_PROXY=localhost", + })) +} diff --git a/util/network/proxyprovider/provider_linux.go b/util/network/proxyprovider/provider_linux.go index 0d64382fcdbe..146ff71e7122 100644 --- a/util/network/proxyprovider/provider_linux.go +++ b/util/network/proxyprovider/provider_linux.go @@ -278,14 +278,7 @@ func (n *proxyNS) Sample() (*resourcestypes.NetworkSample, error) { func (n *proxyNS) ProxyEnv() []string { proxy := "http://" + n.ln.Addr().String() noProxy := "127.0.0.1,localhost,::1" - return []string{ - "HTTP_PROXY=" + proxy, - "HTTPS_PROXY=" + proxy, - "http_proxy=" + proxy, - "https_proxy=" + proxy, - "NO_PROXY=" + noProxy, - "no_proxy=" + noProxy, - } + return network.ProxyEnv(proxy, noProxy) } func (n *proxyNS) ProxyCACert() []byte {