diff --git a/act/container/docker_run.go b/act/container/docker_run.go index c5d4e16d..502d4828 100644 --- a/act/container/docker_run.go +++ b/act/container/docker_run.go @@ -42,6 +42,7 @@ import ( "github.com/moby/moby/api/types/system" "github.com/moby/moby/client" specs "github.com/opencontainers/image-spec/specs-go/v1" + "github.com/sirupsen/logrus" ) // drainGracePeriod bounds how long we wait for an output-copy goroutine to @@ -500,6 +501,16 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config return nil, nil, fmt.Errorf("Cannot process container options: '%s': '%w'", input.Options, err) } + // For Gitea + // When privileged mode is disabled, container.options is workflow-controlled + // untrusted input. Strip the HostConfig fields that would let a workflow break + // out of the container (host namespaces, capability expansion, security profile + // overrides, device and runtime access). Otherwise these survive into the final + // HostConfig even though --privileged is forced off. + if !hostConfig.Privileged { + sanitizeOptionsHostConfig(logger, containerConfig.HostConfig) + } + logger.Debugf("Custom container.Config from options ==> %+v", containerConfig.Config) err = mergo.Merge(config, containerConfig.Config, mergo.WithOverride, mergo.WithAppendSlice) @@ -1101,6 +1112,77 @@ func (cr *containerReference) wait() common.Executor { } } +// For Gitea +// sanitizeOptionsHostConfig clears the HostConfig fields parsed from a +// workflow-controlled container.options string that could be used to escape the +// container when privileged mode is disabled. It must only be called when the +// runner has privileged mode turned off; with privileged mode enabled the +// administrator has already opted into host access. +func sanitizeOptionsHostConfig(logger logrus.FieldLogger, hostConfig *container.HostConfig) { + warn := func(option string) { + logger.Warnf("container option %q is not allowed when privileged mode is disabled and will be ignored", option) + } + + if hostConfig.PidMode != "" { + warn("--pid") + hostConfig.PidMode = "" + } + if hostConfig.IpcMode != "" { + warn("--ipc") + hostConfig.IpcMode = "" + } + if hostConfig.UTSMode != "" { + warn("--uts") + hostConfig.UTSMode = "" + } + if hostConfig.CgroupnsMode != "" { + warn("--cgroupns") + hostConfig.CgroupnsMode = "" + } + // UsernsMode is set from the runner-controlled input; never let options + // override it (e.g. --userns=host disables user namespace remapping). + if hostConfig.UsernsMode != "" { + warn("--userns") + hostConfig.UsernsMode = "" + } + if len(hostConfig.CapAdd) > 0 { + warn("--cap-add") + hostConfig.CapAdd = nil + } + if len(hostConfig.SecurityOpt) > 0 { + warn("--security-opt") + hostConfig.SecurityOpt = nil + } + if len(hostConfig.Devices) > 0 { + warn("--device") + hostConfig.Devices = nil + } + if len(hostConfig.DeviceCgroupRules) > 0 { + warn("--device-cgroup-rule") + hostConfig.DeviceCgroupRules = nil + } + if len(hostConfig.DeviceRequests) > 0 { + warn("--gpus") + hostConfig.DeviceRequests = nil + } + if len(hostConfig.VolumesFrom) > 0 { + warn("--volumes-from") + hostConfig.VolumesFrom = nil + } + if hostConfig.Runtime != "" { + warn("--runtime") + hostConfig.Runtime = "" + } + if hostConfig.CgroupParent != "" { + warn("--cgroup-parent") + hostConfig.CgroupParent = "" + } + if len(hostConfig.Sysctls) > 0 { + warn("--sysctl") + hostConfig.Sysctls = nil + } +} + // For Gitea // sanitizeConfig remove the invalid configurations from `config` and `hostConfig` func (cr *containerReference) sanitizeConfig(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig) { diff --git a/act/container/docker_run_test.go b/act/container/docker_run_test.go index cb353f4e..bc556509 100644 --- a/act/container/docker_run_test.go +++ b/act/container/docker_run_test.go @@ -669,6 +669,110 @@ func TestCheckVolumes(t *testing.T) { } } +func TestSanitizeOptionsHostConfig(t *testing.T) { + logger, _ := test.NewNullLogger() + + dangerous := func() *container.HostConfig { + return &container.HostConfig{ + PidMode: "host", + IpcMode: "host", + UTSMode: "host", + CgroupnsMode: "host", + UsernsMode: "host", + CapAdd: []string{"ALL"}, + SecurityOpt: []string{"seccomp=unconfined", "apparmor=unconfined"}, + VolumesFrom: []string{"other"}, + Runtime: "runc", + Resources: container.Resources{ + CgroupParent: "/custom", + Devices: []container.DeviceMapping{{PathOnHost: "/dev/sda", PathInContainer: "/dev/sda", CgroupPermissions: "rwm"}}, + DeviceCgroupRules: []string{"a *:* rwm"}, + }, + Sysctls: map[string]string{"net.ipv4.ip_forward": "1"}, + } + } + + hostConfig := dangerous() + sanitizeOptionsHostConfig(logger, hostConfig) + + assert.Empty(t, string(hostConfig.PidMode)) + assert.Empty(t, string(hostConfig.IpcMode)) + assert.Empty(t, string(hostConfig.UTSMode)) + assert.Empty(t, string(hostConfig.CgroupnsMode)) + assert.Empty(t, string(hostConfig.UsernsMode)) + assert.Empty(t, hostConfig.CapAdd) + assert.Empty(t, hostConfig.SecurityOpt) + assert.Empty(t, hostConfig.Devices) + assert.Empty(t, hostConfig.DeviceCgroupRules) + assert.Empty(t, hostConfig.VolumesFrom) + assert.Empty(t, hostConfig.Runtime) + assert.Empty(t, hostConfig.CgroupParent) + assert.Empty(t, hostConfig.Sysctls) +} + +func TestMergeContainerConfigsStripsDangerousOptionsWhenUnprivileged(t *testing.T) { + // OS-independent options only: --device parsing requires a linux/windows + // server OS, which is not guaranteed for the test host. + const dangerousOptions = "--pid=host --ipc=host --uts=host --cgroupns=host " + + "--userns=host --cap-add=ALL --security-opt seccomp=unconfined " + + "--security-opt apparmor=unconfined --volumes-from other " + + "--runtime runc --cgroup-parent /custom --sysctl net.ipv4.ip_forward=1" + + t.Run("unprivileged strips host-escape options", func(t *testing.T) { + logger, _ := test.NewNullLogger() + ctx := common.WithLogger(context.Background(), logger) + cr := &containerReference{ + input: &NewContainerInput{ + Options: dangerousOptions, + NetworkMode: "bridge", + UsernsMode: "private", + }, + } + + _, hostConfig, err := cr.mergeContainerConfigs(ctx, &container.Config{}, &container.HostConfig{ + Privileged: false, + UsernsMode: container.UsernsMode("private"), + NetworkMode: container.NetworkMode("bridge"), + }) + require.NoError(t, err) + + assert.False(t, hostConfig.Privileged) + assert.Empty(t, string(hostConfig.PidMode)) + assert.Empty(t, string(hostConfig.IpcMode)) + assert.Empty(t, string(hostConfig.UTSMode)) + assert.Empty(t, string(hostConfig.CgroupnsMode)) + // UsernsMode must keep the runner-controlled value, not the one from options. + assert.Equal(t, "private", string(hostConfig.UsernsMode)) + assert.Empty(t, hostConfig.CapAdd) + assert.Empty(t, hostConfig.SecurityOpt) + assert.Empty(t, hostConfig.VolumesFrom) + assert.Empty(t, hostConfig.Runtime) + assert.Empty(t, hostConfig.CgroupParent) + assert.Empty(t, hostConfig.Sysctls) + }) + + t.Run("privileged preserves options", func(t *testing.T) { + logger, _ := test.NewNullLogger() + ctx := common.WithLogger(context.Background(), logger) + cr := &containerReference{ + input: &NewContainerInput{ + Options: "--pid=host --cap-add=ALL --security-opt seccomp=unconfined", + NetworkMode: "bridge", + }, + } + + _, hostConfig, err := cr.mergeContainerConfigs(ctx, &container.Config{}, &container.HostConfig{ + Privileged: true, + NetworkMode: container.NetworkMode("bridge"), + }) + require.NoError(t, err) + + assert.Equal(t, "host", string(hostConfig.PidMode)) + assert.Equal(t, []string{"ALL"}, hostConfig.CapAdd) + assert.Equal(t, []string{"seccomp=unconfined"}, hostConfig.SecurityOpt) + }) +} + func TestCheckVolumesRejectsEscapingHostPaths(t *testing.T) { logger, _ := test.NewNullLogger() ctx := common.WithLogger(context.Background(), logger)