fix: keep the runner's own container.options when privileged is off (#1151)

The runner's own `container.options` and the workflow's were joined into one string before parsing, so the host-escape filter added in https://gitea.com/gitea/runner/pulls/1058 dropped the administrator's options along with the workflow's. Setups that need `--device` or `--security-opt` from the config file had no way left to get them short of enabling privileged mode.

`NewContainerInput` now carries the two sources apart, as `RunnerOptions` and `WorkflowOptions`, down to the point where the filter runs. With privileged mode off, the host-escape fields are reset to what the runner's own options parse to on their own, so only the workflow's contribution is dropped.

Three further ways a workflow's options reached past its container, all resolved on the runner before anything reaches the daemon:

1. `--env-file` and `--label-file` name files that are read on the runner, so any file it could read became container environment or labels. Both are refused from a workflow now, and still serve the runner's own options.
2. A bare `--env NAME` was resolved from the runner's own environment by docker's validator. That lookup is gone, for every source. Use `runner.envs` or `runner.env_file` to pass a variable on.
3. A volume driver decides for itself what it mounts, and the local driver's `device=` option turns a name `valid_volumes` allows into a bind of any host path. A workflow's mounts may no longer carry one.

`--isolation`, `--volume-driver` and the two paths `--security-opt systempaths=unconfined` lands in were also missing from the fields a workflow may not set.

Last, the `--network and --net in the options will be ignored.` warning fired for every container, because the runner's own network mode is fed into the parsed options before the check runs.

Fixes https://gitea.com/gitea/runner/issues/1142

Reviewed-on: https://gitea.com/gitea/runner/pulls/1151
Reviewed-by: bircni <bircni@icloud.com>
Co-authored-by: silverwind <me@silverwind.io>

act/container/container_types.go

@@ -25,26 +25,27 @@ func (e ExitCodeError) Error() string {
 
 // NewContainerInput the input for the New function
 type NewContainerInput struct {
-	Image          string
+	Image           string
-	Username       string
+	Username        string
-	Password       string
+	Password        string
-	Entrypoint     []string
+	Entrypoint      []string
-	Cmd            []string
+	Cmd             []string
-	WorkingDir     string
+	WorkingDir      string
-	Env            []string
+	Env             []string
-	Binds          []string
+	Binds           []string
-	Mounts         map[string]string
+	Mounts          map[string]string
-	Name           string
+	Name            string
-	Stdout         io.Writer
+	Stdout          io.Writer
-	Stderr         io.Writer
+	Stderr          io.Writer
-	NetworkMode    string
+	NetworkMode     string
-	Privileged     bool
+	Privileged      bool
-	UsernsMode     string
+	UsernsMode      string
-	Platform       string
+	Platform        string
-	Options        string
+	RunnerOptions   string // container options the runner was configured with, trusted
-	NetworkAliases []string
+	WorkflowOptions string // container options the workflow asked for, untrusted
-	ExposedPorts   nat.PortSet
+	NetworkAliases  []string
-	PortBindings   nat.PortMap
+	ExposedPorts    nat.PortSet
+	PortBindings    nat.PortMap
 
 	// Gitea specific
 	AutoRemove   bool

act/container/docker_cli.go

@@ -350,7 +350,7 @@ type containerConfig struct {
 // parse parses the args for the specified command and generates a Config,
 // a HostConfig and returns them with the specified command.
 // If the specified args are not valid, it will return an error.
-func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*containerConfig, error) { //nolint:gocyclo // verbatim copy from docker/cli
+func parse(flags *pflag.FlagSet, copts *containerOptions, serverOS string) (*containerConfig, error) { //nolint:gocyclo,unparam // verbatim copy from docker/cli
 	var (
 		attachStdin  = copts.attach.Get("stdin")
 		attachStdout = copts.attach.Get("stdout")

act/container/docker_create_flags.go

@@ -10,7 +10,9 @@ import (
 	"fmt"
 	"io"
 	"slices"
+	"strings"
 
+	"github.com/docker/cli/opts"
 	"github.com/kballard/go-shellquote"
 	"github.com/spf13/pflag"
 )
@@ -51,6 +53,7 @@ func parseContainerOptions(options string) (*pflag.FlagSet, *containerOptions, *
 	flags := pflag.NewFlagSet("container_flags", pflag.ContinueOnError)
 	flags.SetOutput(io.Discard)
 	copts := addFlags(flags)
+	copts.env = opts.NewListOpts(validateEnv) // addFlags registered this field's address, so the swap takes effect
 	cf := registerCreateFlags(flags)
 
 	args, err := shellquote.Split(options)
@@ -73,6 +76,30 @@ func createFlagsFromOptions(options string) *createFlags {
 	return cf
 }
 
+// validateEnv is opts.ValidateEnv without its lookup of a bare name in the runner's environment.
+func validateEnv(val string) (string, error) {
+	if name, _, _ := strings.Cut(val, "="); name == "" {
+		return "", errors.New("invalid environment variable: " + val)
+	}
+	return val, nil
+}
+
+// rejectHostReadingOptions refuses the flags naming files that are read here, on the
+// runner, rather than in the container.
+func rejectHostReadingOptions(options string) error {
+	flags, _, _, err := parseContainerOptions(options)
+	if err != nil {
+		return err
+	}
+
+	for _, name := range []string{"env-file", "label-file"} {
+		if flags.Changed(name) {
+			return fmt.Errorf("container option --%s reads files from the runner and is not allowed in a workflow", name)
+		}
+	}
+	return nil
+}
+
 func (cf *createFlags) validate() error {
 	if !slices.Contains(pullPolicies, cf.pull) {
 		return fmt.Errorf("invalid --pull option %q: must be one of %q", cf.pull, pullPolicies)

act/container/docker_create_flags_test.go

@@ -50,13 +50,13 @@ func TestCreateFlagsValidate(t *testing.T) {
 }
 
 func TestNewContainerAppliesCreateFlags(t *testing.T) {
-	input := &NewContainerInput{Platform: "linux/amd64", Options: "--platform linux/arm64 --pull never"}
+	input := &NewContainerInput{Platform: "linux/amd64", RunnerOptions: "--pull never", WorkflowOptions: "--platform linux/arm64"}
 	cr, ok := NewContainer(input).(*containerReference)
 	require.True(t, ok)
 	assert.Equal(t, "linux/arm64", input.Platform)
 	assert.Equal(t, pullPolicyNever, cr.pullPolicy)
 
-	kept := &NewContainerInput{Platform: "linux/amd64", Options: "--privileged"}
+	kept := &NewContainerInput{Platform: "linux/amd64", RunnerOptions: "--privileged"}
 	NewContainer(kept)
 	assert.Equal(t, "linux/amd64", kept.Platform)
 }

act/container/docker_run.go

@@ -15,6 +15,7 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"reflect"
 	"regexp"
 	"runtime"
 	"slices"
@@ -57,7 +58,7 @@ func NewContainer(input *NewContainerInput) ExecutionsEnvironment {
 	cr := new(containerReference)
 	cr.input = input
 	// Resolved up front because the image pull runs before the container is created.
-	cf := createFlagsFromOptions(input.Options)
+	cf := createFlagsFromOptions(input.allOptions())
 	if cf.platform != "" {
 		cr.input.Platform = cf.platform
 	}
@@ -524,22 +525,32 @@ func (cr *containerReference) waitForRemoval(ctx context.Context, idOrName strin
 	}
 }
 
+// allOptions puts the runner's options first, so a flag both sources set ends up the workflow's.
+func (input *NewContainerInput) allOptions() string {
+	return strings.TrimSpace(input.RunnerOptions + " " + input.WorkflowOptions)
+}
+
 func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config *container.Config, hostConfig *container.HostConfig) (*container.Config, *container.HostConfig, error) {
 	logger := common.Logger(ctx)
-	input := cr.input
+	options := cr.input.allOptions()
 
-	if input.Options == "" {
+	if options == "" {
 		return config, hostConfig, nil
 	}
 
+	// For Gitea, checked here because the parse below is what would read those files
+	if err := rejectHostReadingOptions(cr.input.WorkflowOptions); err != nil {
+		return nil, nil, err
+	}
+
 	// parse configuration from CLI container.options
-	flags, copts, cf, err := parseContainerOptions(input.Options)
+	flags, copts, cf, err := parseContainerOptions(options)
 	if err != nil {
 		return nil, nil, err
 	}
 
 	if err := cf.validate(); err != nil {
-		return nil, nil, fmt.Errorf("cannot process container options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
 	}
 
 	// FIXME: If everything is fine after gitea/act v0.260.0, remove the following comment.
@@ -570,24 +581,23 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 
 	containerConfig, err := parse(flags, copts, runtime.GOOS)
 	if err != nil {
-		return nil, nil, fmt.Errorf("cannot process container options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
 	}
 
-	// For Gitea
+	// For Gitea, forcing --privileged off is not enough, other options reach the host too
-	// 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)
+		trusted, err := parseOptionsHostConfig(cr.input.RunnerOptions)
+		if err != nil {
+			return nil, nil, err
+		}
+		sanitizeOptionsHostConfig(logger, containerConfig.HostConfig, trusted)
 	}
 
 	logger.Debugf("Custom container.Config from options ==> %+v", containerConfig.Config)
 
 	err = mergo.Merge(config, containerConfig.Config, mergo.WithOverride, mergo.WithAppendSlice)
 	if err != nil {
-		return nil, nil, fmt.Errorf("cannot merge container.Config options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("cannot merge container.Config options: '%s': '%w'", options, err)
 	}
 	logger.Debugf("Merged container.Config ==> %+v", config)
 
@@ -599,14 +609,15 @@ func (cr *containerReference) mergeContainerConfigs(ctx context.Context, config
 	networkMode := hostConfig.NetworkMode
 	err = mergo.Merge(hostConfig, containerConfig.HostConfig, mergo.WithOverride)
 	if err != nil {
-		return nil, nil, fmt.Errorf("cannot merge container.HostConfig options: '%s': '%w'", input.Options, err)
+		return nil, nil, fmt.Errorf("cannot merge container.HostConfig options: '%s': '%w'", options, err)
 	}
 	hostConfig.Binds = binds
 	hostConfig.Mounts = mounts
 	if cf.name != "" {
 		logger.Warn("--name in the options will be ignored.")
 	}
-	if len(copts.netMode.Value()) > 0 {
+	// the runner's own network mode was put into copts above, so ask the flags instead
+	if flags.Changed("network") || flags.Changed("net") {
 		logger.Warn("--network and --net in the options will be ignored.")
 	}
 	hostConfig.NetworkMode = networkMode
@@ -1112,74 +1123,64 @@ func (cr *containerReference) wait() common.Executor {
 }
 
 // For Gitea
-// sanitizeOptionsHostConfig clears the HostConfig fields parsed from a
+// sanitizeOptionsHostConfig takes back everything a workflow could escape the container with,
-// workflow-controlled container.options string that could be used to escape the
+// setting each field to trusted, which is what the runner's own options parse to on their own.
-// container when privileged mode is disabled. It must only be called when the
+// Only for unprivileged mode, since privileged mode grants host access anyway.
-// runner has privileged mode turned off; with privileged mode enabled the
+func sanitizeOptionsHostConfig(logger logrus.FieldLogger, hostConfig, trusted *container.HostConfig) {
-// administrator has already opted into host access.
+	resetOption(logger, "--pid", &hostConfig.PidMode, trusted.PidMode)
-func sanitizeOptionsHostConfig(logger logrus.FieldLogger, hostConfig *container.HostConfig) {
+	resetOption(logger, "--ipc", &hostConfig.IpcMode, trusted.IpcMode)
-	warn := func(option string) {
+	resetOption(logger, "--uts", &hostConfig.UTSMode, trusted.UTSMode)
-		logger.Warnf("container option %q is not allowed when privileged mode is disabled and will be ignored", option)
+	resetOption(logger, "--cgroupns", &hostConfig.CgroupnsMode, trusted.CgroupnsMode)
-	}
+	resetOption(logger, "--userns", &hostConfig.UsernsMode, trusted.UsernsMode) // --userns=host would undo the remapping the runner asked for
+	resetOption(logger, "--cap-add", &hostConfig.CapAdd, trusted.CapAdd)
+	resetOption(logger, "--security-opt", &hostConfig.SecurityOpt, trusted.SecurityOpt)
+	resetOption(logger, "--device", &hostConfig.Devices, trusted.Devices)
+	resetOption(logger, "--device-cgroup-rule", &hostConfig.DeviceCgroupRules, trusted.DeviceCgroupRules)
+	resetOption(logger, "--gpus", &hostConfig.DeviceRequests, trusted.DeviceRequests)
+	resetOption(logger, "--volumes-from", &hostConfig.VolumesFrom, trusted.VolumesFrom)
+	resetOption(logger, "--runtime", &hostConfig.Runtime, trusted.Runtime)
+	resetOption(logger, "--cgroup-parent", &hostConfig.CgroupParent, trusted.CgroupParent)
+	resetOption(logger, "--sysctl", &hostConfig.Sysctls, trusted.Sysctls)
+	resetOption(logger, "--isolation", &hostConfig.Isolation, trusted.Isolation) // windows: process isolation drops the hyper-v boundary
+	resetOption(logger, "--volume-driver", &hostConfig.VolumeDriver, trusted.VolumeDriver)
+	// systempaths=unconfined lands in these two rather than in SecurityOpt
+	resetOption(logger, "--security-opt", &hostConfig.MaskedPaths, trusted.MaskedPaths)
+	resetOption(logger, "--security-opt", &hostConfig.ReadonlyPaths, trusted.ReadonlyPaths)
 
-	if hostConfig.PidMode != "" {
+	// a driver mounts what it likes, e.g. local with device= binds any host path, which
-		warn("--pid")
+	// valid_volumes never gets to see
-		hostConfig.PidMode = ""
+	hostConfig.Mounts = slices.DeleteFunc(hostConfig.Mounts, func(mt mount.Mount) bool {
+		if mt.VolumeOptions == nil || mt.VolumeOptions.DriverConfig == nil ||
+			slices.ContainsFunc(trusted.Mounts, func(t mount.Mount) bool { return reflect.DeepEqual(t, mt) }) {
+			return false
+		}
+		logger.Warnf("volume driver of %q in the workflow is not allowed when privileged mode is disabled and will be ignored", mt.Source)
+		return true
+	})
+}
+
+// resetOption puts a field back to the runner's own value. It compares the values rather than
+// the flags, so a field that more than one option feeds cannot slip through.
+func resetOption[T any](logger logrus.FieldLogger, option string, field *T, trusted T) {
+	if reflect.DeepEqual(*field, trusted) {
+		return
 	}
-	if hostConfig.IpcMode != "" {
+	logger.Warnf("container option %q in the workflow is not allowed when privileged mode is disabled and will be ignored", option)
-		warn("--ipc")
+	*field = trusted
-		hostConfig.IpcMode = ""
+}
+
+// parseOptionsHostConfig parses one options string on its own, to see what it alone asks for.
+// Even "" goes through the parser, or its empty slices and maps would differ from a real parse.
+func parseOptionsHostConfig(options string) (*container.HostConfig, error) {
+	flags, copts, _, err := parseContainerOptions(options)
+	if err != nil {
+		return nil, err
 	}
-	if hostConfig.UTSMode != "" {
+	containerConfig, err := parse(flags, copts, runtime.GOOS)
-		warn("--uts")
+	if err != nil {
-		hostConfig.UTSMode = ""
+		return nil, fmt.Errorf("cannot process container options: '%s': '%w'", options, err)
-	}
-	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
 	}
+	return containerConfig.HostConfig, nil
 }
 
 // For Gitea

act/container/docker_run_test.go

@@ -583,106 +583,133 @@ func TestCheckVolumes(t *testing.T) {
 	}
 }
 
+// A volume driver decides for itself what it mounts, e.g. the local driver with device= binds
+// any host path, which valid_volumes never gets to see.
+func TestMergeContainerConfigsDropsVolumeDriversFromWorkflows(t *testing.T) {
+	const escape = "--mount type=volume,src=job-escape,dst=/host,volume-driver=local,volume-opt=type=none,volume-opt=o=bind,volume-opt=device=/"
+
+	hostConfig, _ := mergeOptions(t, "", escape+" --mount type=volume,src=job-plain,dst=/cache", false)
+	require.Len(t, hostConfig.Mounts, 1)
+	assert.Equal(t, "job-plain", hostConfig.Mounts[0].Source)
+
+	// the same mount from the runner's own options is the administrator's to make
+	hostConfig, _ = mergeOptions(t, escape, "", false)
+	require.Len(t, hostConfig.Mounts, 1)
+	assert.Equal(t, "job-escape", hostConfig.Mounts[0].Source)
+}
+
+// Both of these are read here, on the runner, so a workflow could read the runner's files
+// and environment with them.
+func TestMergeContainerConfigsKeepsTheRunnersFilesAndEnvToItself(t *testing.T) {
+	hostFile := filepath.Join(t.TempDir(), "host.env")
+	require.NoError(t, os.WriteFile(hostFile, []byte("STOLEN=from-the-host\n"), 0o600))
+	t.Setenv("RUNNER_SECRET", "s3cr3t")
+
+	for _, option := range []string{"--env-file " + hostFile, "--label-file " + hostFile} {
+		logger, _ := test.NewNullLogger()
+		cr := &containerReference{input: &NewContainerInput{NetworkMode: "bridge", WorkflowOptions: option}}
+
+		_, _, err := cr.mergeContainerConfigs(common.WithLogger(context.Background(), logger), &container.Config{}, &container.HostConfig{})
+		require.ErrorContains(t, err, "not allowed in a workflow")
+
+		// the runner reading its own files is what those options are for
+		cr = &containerReference{input: &NewContainerInput{NetworkMode: "bridge", RunnerOptions: option}}
+		_, _, err = cr.mergeContainerConfigs(common.WithLogger(context.Background(), logger), &container.Config{}, &container.HostConfig{})
+		require.NoError(t, err)
+	}
+
+	// a bare name is no longer resolved from the runner's environment, for either source
+	logger, _ := test.NewNullLogger()
+	cr := &containerReference{input: &NewContainerInput{
+		NetworkMode:     "bridge",
+		RunnerOptions:   "--env RUNNER_SECRET",
+		WorkflowOptions: "--env RUNNER_SECRET --env GIVEN=value",
+	}}
+
+	config, _, err := cr.mergeContainerConfigs(common.WithLogger(context.Background(), logger), &container.Config{}, &container.HostConfig{})
+	require.NoError(t, err)
+	assert.Equal(t, []string{"RUNNER_SECRET", "RUNNER_SECRET", "GIVEN=value"}, config.Env)
+}
+
 func TestSanitizeOptionsHostConfig(t *testing.T) {
 	logger, _ := test.NewNullLogger()
 
-	dangerous := func() *container.HostConfig {
+	// every field the sanitizer resets, so a reset dropped in a refactor fails here
-		return &container.HostConfig{
+	hostConfig := &container.HostConfig{
-			PidMode:           "host",
+		PidMode:           "host",
-			IpcMode:           "host",
+		IpcMode:           "host",
-			UTSMode:           "host",
+		UTSMode:           "host",
-			CgroupnsMode:      "host",
+		CgroupnsMode:      "host",
-			UsernsMode:        "host",
+		UsernsMode:        "host",
-			CapAdd:            []string{"ALL"},
+		CapAdd:            []string{"ALL"},
-			SecurityOpt:       []string{"seccomp=unconfined", "apparmor=unconfined"},
+		SecurityOpt:       []string{"seccomp=unconfined", "apparmor=unconfined"},
-			VolumesFrom:       []string{"other"},
+		VolumesFrom:       []string{"other"},
-			Runtime:           "runc",
+		Runtime:           "runc",
-			CgroupParent:      "/custom",
+		Isolation:         "process",
-			Devices:           []container.DeviceMapping{{PathOnHost: "/dev/sda", PathInContainer: "/dev/sda", CgroupPermissions: "rwm"}},
+		VolumeDriver:      "rogue",
-			DeviceCgroupRules: []string{"a *:* rwm"},
+		MaskedPaths:       []string{},
-			Sysctls:           map[string]string{"net.ipv4.ip_forward": "1"},
+		ReadonlyPaths:     []string{},
-		}
+		CgroupParent:      "/custom",
+		Devices:           []container.DeviceMapping{{PathOnHost: "/dev/sda", PathInContainer: "/dev/sda", CgroupPermissions: "rwm"}},
+		DeviceCgroupRules: []string{"a *:* rwm"},
+		DeviceRequests:    []container.DeviceRequest{{Count: -1, Capabilities: [][]string{{"gpu"}}}},
+		Sysctls:           map[string]string{"net.ipv4.ip_forward": "1"},
 	}
 
-	hostConfig := dangerous()
+	sanitizeOptionsHostConfig(logger, hostConfig, &container.HostConfig{})
-	sanitizeOptionsHostConfig(logger, hostConfig)
 
-	assert.Empty(t, string(hostConfig.PidMode))
+	assert.Equal(t, &container.HostConfig{}, hostConfig)
-	assert.Empty(t, string(hostConfig.IpcMode))
+}
-	assert.Empty(t, string(hostConfig.UTSMode))
+
-	assert.Empty(t, string(hostConfig.CgroupnsMode))
+// mergeOptions merges both option sources into a bare container, returning the result and its log.
-	assert.Empty(t, string(hostConfig.UsernsMode))
+func mergeOptions(t *testing.T, runnerOptions, workflowOptions string, privileged bool) (*container.HostConfig, *test.Hook) {
-	assert.Empty(t, hostConfig.CapAdd)
+	t.Helper()
-	assert.Empty(t, hostConfig.SecurityOpt)
+	logger, hook := test.NewNullLogger()
-	assert.Empty(t, hostConfig.Devices)
+	cr := &containerReference{input: &NewContainerInput{
-	assert.Empty(t, hostConfig.DeviceCgroupRules)
+		RunnerOptions:   runnerOptions,
-	assert.Empty(t, hostConfig.VolumesFrom)
+		WorkflowOptions: workflowOptions,
-	assert.Empty(t, hostConfig.Runtime)
+		NetworkMode:     "bridge",
-	assert.Empty(t, hostConfig.CgroupParent)
+		UsernsMode:      "private",
-	assert.Empty(t, hostConfig.Sysctls)
+	}}
+
+	_, hostConfig, err := cr.mergeContainerConfigs(common.WithLogger(context.Background(), logger), &container.Config{}, &container.HostConfig{
+		Privileged:  privileged,
+		UsernsMode:  container.UsernsMode("private"),
+		NetworkMode: container.NetworkMode("bridge"),
+	})
+	require.NoError(t, err)
+	return hostConfig, hook
 }
 
 func TestMergeContainerConfigsStripsDangerousOptionsWhenUnprivileged(t *testing.T) {
-	// OS-independent options only: --device parsing requires a linux/windows
+	// OS-independent options only, --device and --gpus need a linux/windows server OS
-	// 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 " +
+		"--security-opt apparmor=unconfined --volumes-from other --isolation process " +
 		"--runtime runc --cgroup-parent /custom --sysctl net.ipv4.ip_forward=1"
 
-	t.Run("unprivileged strips host-escape options", func(t *testing.T) {
+	// whatever the workflow adds, an unprivileged container comes out exactly as the runner's
-		logger, _ := test.NewNullLogger()
+	// own options alone describe it, field for field
-		ctx := common.WithLogger(context.Background(), logger)
+	for _, runnerOptions := range []string{"--shm-size 1g", dangerousOptions, "--cap-add SYS_ADMIN --security-opt seccomp=unconfined"} {
-		cr := &containerReference{
+		runnerOnly, _ := mergeOptions(t, runnerOptions, "", false)
-			input: &NewContainerInput{
+		withWorkflow, _ := mergeOptions(t, runnerOptions, dangerousOptions, false)
-				Options:     dangerousOptions,
-				NetworkMode: "bridge",
-				UsernsMode:  "private",
-			},
-		}
 
-		_, hostConfig, err := cr.mergeContainerConfigs(ctx, &container.Config{}, &container.HostConfig{
+		assert.Equal(t, runnerOnly, withWorkflow, "runner options: %q", runnerOptions)
-			Privileged:  false,
+	}
-			UsernsMode:  container.UsernsMode("private"),
-			NetworkMode: container.NetworkMode("bridge"),
-		})
-		require.NoError(t, err)
 
-		assert.False(t, hostConfig.Privileged)
+	// the same options from the runner reach the daemon, even --userns, which no workflow may set
-		assert.Empty(t, string(hostConfig.PidMode))
+	kept, _ := mergeOptions(t, dangerousOptions, "", false)
-		assert.Empty(t, string(hostConfig.IpcMode))
+	assert.Equal(t, "host", string(kept.PidMode))
-		assert.Empty(t, string(hostConfig.UTSMode))
+	assert.Equal(t, []string{"ALL"}, kept.CapAdd)
-		assert.Empty(t, string(hostConfig.CgroupnsMode))
+	assert.Equal(t, "runc", kept.Runtime)
-		// UsernsMode must keep the runner-controlled value, not the one from options.
+	assert.Equal(t, "host", string(kept.UsernsMode))
-		assert.Equal(t, "private", string(hostConfig.UsernsMode))
+	assert.False(t, kept.Privileged)
-		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) {
+	// privileged is the administrator opting in, so the workflow's options are honored
-		logger, _ := test.NewNullLogger()
+	privileged, _ := mergeOptions(t, "", dangerousOptions, true)
-		ctx := common.WithLogger(context.Background(), logger)
+	assert.Equal(t, "host", string(privileged.PidMode))
-		cr := &containerReference{
+	assert.Equal(t, []string{"ALL"}, privileged.CapAdd)
-			input: &NewContainerInput{
+	assert.Equal(t, []string{"seccomp=unconfined", "apparmor=unconfined"}, privileged.SecurityOpt)
-				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) {
@@ -780,8 +807,8 @@ func TestMergeContainerConfigsVolumesReplaceRunnerMounts(t *testing.T) {
 	ctx := common.WithLogger(context.Background(), logger)
 	cr := &containerReference{
 		input: &NewContainerInput{
-			NetworkMode: "bridge",
+			NetworkMode:   "bridge",
-			Options:     "--volume /host/tools:/opt/hostedtoolcache",
+			RunnerOptions: "--volume /host/tools:/opt/hostedtoolcache",
 		},
 	}
 
@@ -794,6 +821,17 @@ func TestMergeContainerConfigsVolumesReplaceRunnerMounts(t *testing.T) {
 	assert.Empty(t, hostConf.Mounts)
 }
 
+func TestMergeContainerConfigsWarnsOnlyAboutOptionsThatWereGiven(t *testing.T) {
+	warnings := func(runnerOptions, workflowOptions string) int {
+		_, hook := mergeOptions(t, runnerOptions, workflowOptions, false)
+		return len(hook.AllEntries())
+	}
+
+	assert.Zero(t, warnings("--volume /host/tools:/opt/hostedtoolcache", ""))
+	assert.Zero(t, warnings("", "--shm-size 1g"))
+	assert.Equal(t, 1, warnings("--network host", ""))
+}
+
 // A dead daemon must fail the job, not panic through logrus and not silently
 // drop the requested platform.
 func TestSupportsContainerImagePlatformDaemonError(t *testing.T) {

act/runner/action.go

@@ -422,7 +422,7 @@ func evalDockerArgs(ctx context.Context, step step, action *model.Action, cmd *[
 	}
 }
 
-func newStepContainer(ctx context.Context, step step, image string, cmd, entrypoint []string, options string) container.Container {
+func newStepContainer(ctx context.Context, step step, image string, cmd, entrypoint []string, runnerOptions string) container.Container {
 	rc := step.getRunContext()
 	logWriter := rc.commandLogWriter(ctx)
 	envList := make([]string, 0)
@@ -438,24 +438,24 @@ func newStepContainer(ctx context.Context, step step, image string, cmd, entrypo
 		networkMode = "default"
 	}
 	return ContainerNewContainer(&container.NewContainerInput{
-		Cmd:          cmd,
+		Cmd:           cmd,
-		Entrypoint:   entrypoint,
+		Entrypoint:    entrypoint,
-		WorkingDir:   rc.JobContainer.ToContainerPath(rc.Config.Workdir),
+		WorkingDir:    rc.JobContainer.ToContainerPath(rc.Config.Workdir),
-		Image:        image,
+		Image:         image,
-		Name:         createContainerName(rc.jobContainerName(), "STEP-"+step.getStepModel().ID),
+		Name:          createContainerName(rc.jobContainerName(), "STEP-"+step.getStepModel().ID),
-		Env:          envList,
+		Env:           envList,
-		Mounts:       mounts,
+		Mounts:        mounts,
-		NetworkMode:  networkMode,
+		NetworkMode:   networkMode,
-		Binds:        binds,
+		Binds:         binds,
-		Stdout:       logWriter,
+		Stdout:        logWriter,
-		Stderr:       logWriter,
+		Stderr:        logWriter,
-		Privileged:   rc.Config.Privileged,
+		Privileged:    rc.Config.Privileged,
-		UsernsMode:   rc.Config.UsernsMode,
+		UsernsMode:    rc.Config.UsernsMode,
-		Platform:     rc.Config.ContainerArchitecture,
+		Platform:      rc.Config.ContainerArchitecture,
-		Options:      options,
+		RunnerOptions: runnerOptions,
-		AutoRemove:   true,
+		AutoRemove:    true,
-		ValidVolumes: rc.validVolumes(),
+		ValidVolumes:  rc.validVolumes(),
-		AllocatePTY:  rc.Config.AllocatePTY,
+		AllocatePTY:   rc.Config.AllocatePTY,
 	})
 }
 

act/runner/run_context.go

@@ -497,27 +497,27 @@ func (rc *RunContext) startJobContainer() common.Executor {
 
 			serviceContainerName := createContainerName(rc.jobContainerName(), serviceID)
 			c := newContainer(&container.NewContainerInput{
-				Name:           serviceContainerName,
+				Name:            serviceContainerName,
-				WorkingDir:     ext.ToContainerPath(rc.Config.Workdir),
+				WorkingDir:      ext.ToContainerPath(rc.Config.Workdir),
-				Image:          serviceImage,
+				Image:           serviceImage,
-				Username:       serviceUsername,
+				Username:        serviceUsername,
-				Password:       servicePassword,
+				Password:        servicePassword,
-				Cmd:            interpolatedCmd,
+				Cmd:             interpolatedCmd,
-				Env:            envs,
+				Env:             envs,
-				Mounts:         serviceMounts,
+				Mounts:          serviceMounts,
-				Binds:          serviceBinds,
+				Binds:           serviceBinds,
-				Stdout:         logWriter,
+				Stdout:          logWriter,
-				Stderr:         logWriter,
+				Stderr:          logWriter,
-				Privileged:     rc.Config.Privileged,
+				Privileged:      rc.Config.Privileged,
-				UsernsMode:     rc.Config.UsernsMode,
+				UsernsMode:      rc.Config.UsernsMode,
-				Platform:       rc.Config.ContainerArchitecture,
+				Platform:        rc.Config.ContainerArchitecture,
-				AutoRemove:     false, // so a dead service's log survives, cleanupJobResources removes it
+				AutoRemove:      false, // so a dead service's log survives, cleanupJobResources removes it
-				Options:        rc.ExprEval.Interpolate(ctx, spec.Options),
+				WorkflowOptions: rc.ExprEval.Interpolate(ctx, spec.Options),
-				NetworkMode:    networkName,
+				NetworkMode:     networkName,
-				NetworkAliases: []string{serviceID},
+				NetworkAliases:  []string{serviceID},
-				ExposedPorts:   exposedPorts,
+				ExposedPorts:    exposedPorts,
-				PortBindings:   portBindings,
+				PortBindings:    portBindings,
-				AllocatePTY:    rc.Config.AllocatePTY,
+				AllocatePTY:     rc.Config.AllocatePTY,
 			})
 			rc.serviceContainers = append(rc.serviceContainers, &serviceContainer{name: serviceID, image: serviceImage, container: c})
 		}
@@ -528,27 +528,28 @@ func (rc *RunContext) startJobContainer() common.Executor {
 		jobContainerNetwork := networkName
 
 		rc.JobContainer = newContainer(&container.NewContainerInput{
-			Cmd:            nil,
+			Cmd:             nil,
-			Entrypoint:     []string{"/bin/sleep", fmt.Sprint(rc.Config.ContainerMaxLifetime.Round(time.Second).Seconds())},
+			Entrypoint:      []string{"/bin/sleep", fmt.Sprint(rc.Config.ContainerMaxLifetime.Round(time.Second).Seconds())},
-			WorkingDir:     ext.ToContainerPath(rc.Config.Workdir),
+			WorkingDir:      ext.ToContainerPath(rc.Config.Workdir),
-			Image:          image,
+			Image:           image,
-			Username:       username,
+			Username:        username,
-			Password:       password,
+			Password:        password,
-			Name:           name,
+			Name:            name,
-			Env:            envList,
+			Env:             envList,
-			Mounts:         mounts,
+			Mounts:          mounts,
-			NetworkMode:    jobContainerNetwork,
+			NetworkMode:     jobContainerNetwork,
-			NetworkAliases: []string{rc.Name},
+			NetworkAliases:  []string{rc.Name},
-			Binds:          binds,
+			Binds:           binds,
-			Stdout:         logWriter,
+			Stdout:          logWriter,
-			Stderr:         logWriter,
+			Stderr:          logWriter,
-			Privileged:     rc.Config.Privileged,
+			Privileged:      rc.Config.Privileged,
-			UsernsMode:     rc.Config.UsernsMode,
+			UsernsMode:      rc.Config.UsernsMode,
-			Platform:       rc.Config.ContainerArchitecture,
+			Platform:        rc.Config.ContainerArchitecture,
-			Options:        rc.options(ctx),
+			RunnerOptions:   rc.Config.ContainerOptions,
-			AutoRemove:     true,
+			WorkflowOptions: rc.workflowOptions(ctx),
-			ValidVolumes:   rc.validVolumes(),
+			AutoRemove:      true,
-			AllocatePTY:    rc.Config.AllocatePTY,
+			ValidVolumes:    rc.validVolumes(),
+			AllocatePTY:     rc.Config.AllocatePTY,
 		})
 		if rc.JobContainer == nil {
 			return errors.New("failed to create job container")
@@ -1090,14 +1091,13 @@ func (rc *RunContext) platformImage(ctx context.Context) string {
 	return rc.runsOnImage(ctx)
 }
 
-func (rc *RunContext) options(ctx context.Context) string {
+func (rc *RunContext) workflowOptions(ctx context.Context) string {
-	job := rc.Run.Job()
+	c := rc.Run.Job().Container()
-	c := job.Container()
+	if c == nil {
-	if c != nil {
+		return ""
-		return rc.Config.ContainerOptions + " " + rc.ExprEval.Interpolate(ctx, c.Options)
 	}
 
-	return rc.Config.ContainerOptions
+	return rc.ExprEval.Interpolate(ctx, c.Options)
 }
 
 func (rc *RunContext) isEnabled(ctx context.Context) (bool, error) {

act/runner/run_context_test.go

@@ -236,10 +236,48 @@ func (fakeContainer) Inspect(context.Context) (*container.Info, error) {
 
 func (fakeContainer) DumpLogs(context.Context) error { return nil }
 
+// startJobContainerInputs runs startJobContainer against fakeContainer and returns the
+// inputs it built, one per container.
+func startJobContainerInputs(t *testing.T, workflowYAML string, cfg *Config) []*container.NewContainerInput {
+	t.Helper()
+	workflow, err := model.ReadWorkflow(strings.NewReader(workflowYAML))
+	require.NoError(t, err)
+
+	var inputs []*container.NewContainerInput
+	origNewContainer := newContainer
+	newContainer = func(input *container.NewContainerInput) container.ExecutionsEnvironment {
+		inputs = append(inputs, input)
+		return fakeContainer{}
+	}
+	t.Cleanup(func() { newContainer = origNewContainer })
+
+	cfg.Workdir = "/tmp"
+	cfg.ContainerNetworkMode = "host" // an explicit network mode creates no network
+	cfg.Env = map[string]string{}
+	cfg.Secrets = map[string]string{}
+
+	rc := &RunContext{
+		Name:   "test",
+		Config: cfg,
+		Env:    map[string]string{},
+		Run: &model.Run{
+			JobID:    "job",
+			Workflow: workflow,
+		},
+	}
+	rc.ExprEval = rc.NewExpressionEvaluator(t.Context())
+
+	// the inputs are built before the missing daemon fails the first call
+	t.Setenv("DOCKER_HOST", "unix:///nonexistent.sock")
+	require.Error(t, rc.startJobContainer()(t.Context()))
+
+	return inputs
+}
+
 // Regression test: a service without a `credentials:` block resolves to empty
 // credentials, which used to overwrite the job container's own credentials.
 func TestStartJobContainerKeepsJobCredentialsWithServices(t *testing.T) {
-	workflow, err := model.ReadWorkflow(strings.NewReader(`
+	inputs := startJobContainerInputs(t, `
 name: test
 on: push
 jobs:
@@ -259,35 +297,7 @@ jobs:
           username: db-user
           password: db-password
     steps: []
-`))
+`, &Config{})
-	require.NoError(t, err)
-
-	var inputs []*container.NewContainerInput
-	origNewContainer := newContainer
-	newContainer = func(input *container.NewContainerInput) container.ExecutionsEnvironment {
-		inputs = append(inputs, input)
-		return fakeContainer{}
-	}
-	t.Cleanup(func() { newContainer = origNewContainer })
-
-	rc := &RunContext{
-		Name: "test",
-		Config: &Config{
-			Workdir:              "/tmp",
-			ContainerNetworkMode: "host",
-			Env:                  map[string]string{},
-			Secrets:              map[string]string{},
-		},
-		Env: map[string]string{},
-		Run: &model.Run{
-			JobID:    "job",
-			Workflow: workflow,
-		},
-	}
-	rc.ExprEval = rc.NewExpressionEvaluator(t.Context())
-
-	t.Setenv("DOCKER_HOST", "unix:///nonexistent.sock")
-	require.Error(t, rc.startJobContainer()(t.Context()))
 
 	credentials := map[string][2]string{}
 	for _, in := range inputs {
@@ -301,10 +311,39 @@ jobs:
 	require.Equal(t, [2]string{"", ""}, credentials["redis:latest"])
 }
 
+// Only the workflow's options may be stripped later, so the two sources have to reach the
+// container apart from each other.
+func TestStartJobContainerKeepsRunnerOptionsApartFromWorkflowOptions(t *testing.T) {
+	inputs := startJobContainerInputs(t, `
+name: test
+on: push
+jobs:
+  job:
+    runs-on: ubuntu-latest
+    container:
+      image: registry.example/job:latest
+      options: --cap-add SYS_PTRACE
+    services:
+      redis:
+        image: redis:latest
+        options: --shm-size 1g
+    steps: []
+`, &Config{ContainerOptions: "--device /dev/fuse"})
+
+	options := map[string][2]string{}
+	for _, in := range inputs {
+		options[in.Image] = [2]string{in.RunnerOptions, in.WorkflowOptions}
+	}
+
+	require.Equal(t, [2]string{"--device /dev/fuse", "--cap-add SYS_PTRACE"}, options["registry.example/job:latest"])
+	// a service container gets no options from the runner's config today
+	require.Equal(t, [2]string{"", "--shm-size 1g"}, options["redis:latest"])
+}
+
 // A service container reaches the internet the same way the job does, so it inherits the
 // job's proxy; a service that sets the variable itself keeps its own value.
 func TestStartJobContainerGivesServicesTheJobProxy(t *testing.T) {
-	workflow, err := model.ReadWorkflow(strings.NewReader(`
+	inputs := startJobContainerInputs(t, `
 name: test
 on: push
 jobs:
@@ -320,36 +359,7 @@ jobs:
         env:
           no_proxy: db-only.example
     steps: []
-`))
+`, &Config{ProxyEnv: map[string]string{"http_proxy": "http://proxy:3128", "no_proxy": "internal.example"}})
-	require.NoError(t, err)
-
-	var inputs []*container.NewContainerInput
-	origNewContainer := newContainer
-	newContainer = func(input *container.NewContainerInput) container.ExecutionsEnvironment {
-		inputs = append(inputs, input)
-		return fakeContainer{}
-	}
-	t.Cleanup(func() { newContainer = origNewContainer })
-
-	rc := &RunContext{
-		Name: "test",
-		Config: &Config{
-			Workdir:              "/tmp",
-			ContainerNetworkMode: "host",
-			Env:                  map[string]string{},
-			ProxyEnv:             map[string]string{"http_proxy": "http://proxy:3128", "no_proxy": "internal.example"},
-			Secrets:              map[string]string{},
-		},
-		Env: map[string]string{},
-		Run: &model.Run{
-			JobID:    "job",
-			Workflow: workflow,
-		},
-	}
-	rc.ExprEval = rc.NewExpressionEvaluator(t.Context())
-
-	t.Setenv("DOCKER_HOST", "unix:///nonexistent.sock")
-	require.Error(t, rc.startJobContainer()(t.Context()))
 
 	env := map[string][]string{}
 	for _, in := range inputs {

internal/pkg/config/config.example.yaml

@@ -219,6 +219,8 @@ container:
   # A volume declared here replaces the one the runner would mount on the same container path, so the
   # tool cache can be kept on the host. Its source must also be allowed by valid_volumes below:
   #   options: --volume /host/toolcache:/opt/hostedtoolcache
+  # With privileged disabled, options that could escape the container (--security-opt, --device,
+  # --cap-add, --pid, ...) are ignored in a workflow's container.options, but keep working here.
   #options:
   # The parent directory of a job's working directory.
   # NOTE: There is no need to add the first '/' of the path as runner will add it automatically.